Skip to content
This repository has been archived by the owner on Feb 10, 2025. It is now read-only.

feat(netlify): support included/excluded files #325

Closed
wants to merge 13 commits into from
23 changes: 23 additions & 0 deletions .changeset/itchy-donuts-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@astrojs/netlify': minor
---

Adds "includedFiles" and "excludedFiles" options. These allow extra files to be deployed in the SSR function bundle.

When an Astro site using `server` or `hybrid` rendering is deployed to Netlify, the generated functions trace the server dependencies and include any that may be needed in SSR. However sometimes you may want to include extra files that are not detected as dependencies, such as files that are loaded using `fs` functions. Also, you may sometimes want to specifically exclude dependencies that are bundled automatically. For example, you may have a Node module that includes a large binary.

The `includedFiles` and `excludedFiles` options allow you specify these inclusions and exclusions as an array of file paths relative to the project root. Both options support glob patterns, so you can include/exclude multiple files at once.

The paths are relative to the site root. If you are loading them using filesystem functions, make sure you resolve paths relative to the site root, and not relative to the source file. At runtime the compiled file will be in a different location, so paths that are relative to the file will not work. You should instead resolve the path using `path.resolve()` or `process.cwd()`, which will give you the site root.

```js
import netlify from '@astrojs/netlify';
import { defineConfig } from 'astro/config';

export default defineConfig({
output: 'server',
adapter: netlify({
includedFiles: ['src/address-data/**/*.csv', 'src/include-this.txt'],
excludedFiles: ['node_modules/chonky-module/not-this-massive-file.mp4'],
})
});
3 changes: 2 additions & 1 deletion packages/netlify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"files": ["dist"],
"scripts": {
"build": "tsc",
"dev": "tsc --watch",
"test-fn": "astro-scripts test \"test/functions/*.test.js\"",
"test-static": "astro-scripts test \"test/static/*.test.js\"",
"test": "pnpm run test-fn && pnpm run test-static",
Expand All @@ -45,7 +46,7 @@
"@netlify/edge-functions": "^2.8.1",
"@netlify/edge-handler-types": "^0.34.1",
"@types/node": "^20.14.10",
"astro": "^4.11.3",
"astro": "^4.11.5",
"astro-scripts": "workspace:*",
"cheerio": "1.0.0-rc.12",
"execa": "^8.0.1",
Expand Down
39 changes: 35 additions & 4 deletions packages/netlify/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { randomUUID } from 'node:crypto';
import { appendFile, mkdir, readFile, rm, writeFile } from 'node:fs/promises';
import type { IncomingMessage } from 'node:http';
import { fileURLToPath } from 'node:url';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { emptyDir } from '@astrojs/internal-helpers/fs';
import { createRedirectsFromAstroRoutes } from '@astrojs/underscore-redirects';
import type { Context } from '@netlify/functions';
import type { AstroConfig, AstroIntegration, AstroIntegrationLogger, RouteData } from 'astro';
import { build } from 'esbuild';
import glob from 'fast-glob';
import { copyDependenciesToFunction } from './lib/nft.js';
import type { Args } from './ssr-function.js';

const { version: packageVersion } = JSON.parse(
await readFile(new URL('../package.json', import.meta.url), 'utf8')
);
Expand Down Expand Up @@ -185,6 +185,18 @@ export interface NetlifyIntegrationConfig {
* @default {true}
*/
imageCDN?: boolean;

/**
* Extra files to include in the SSR function bundle. Paths are relative to the project root.
* Glob patterns are supported.
*/
includedFiles?: string[];

/**
* Files to exclude from the SSR function bundle. Paths are relative to the project root.
* Glob patterns are supported.
*/
excludedFiles?: string[];
}

export default function netlifyIntegration(
Expand Down Expand Up @@ -237,18 +249,37 @@ export default function netlifyIntegration(
}
}

async function getFilesByGlob(
include: Array<string> = [],
exclude: Array<string> = []
): Promise<Array<URL>> {
const files = await glob(include, {
cwd: fileURLToPath(rootDir),
absolute: true,
ignore: exclude,
});
return files.map((file) => pathToFileURL(file));
}

async function writeSSRFunction({
notFoundContent,
logger,
}: { notFoundContent?: string; logger: AstroIntegrationLogger }) {
const entry = new URL('./entry.mjs', ssrBuildDir());

const includedFiles = await getFilesByGlob(
integrationConfig?.includedFiles,
integrationConfig?.excludedFiles
);

const excludedFiles = await getFilesByGlob(integrationConfig?.excludedFiles);

const { handler } = await copyDependenciesToFunction(
{
entry,
outDir: ssrOutputDir(),
includeFiles: [],
excludeFiles: [],
includeFiles: includedFiles,
excludeFiles: excludedFiles,
logger,
},
TRACE_CACHE
Expand Down
7 changes: 7 additions & 0 deletions packages/netlify/src/ssr-function.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { fileURLToPath } from 'node:url';
import type { Context } from '@netlify/functions';
import type { SSRManifest } from 'astro';
import { App } from 'astro/app';
Expand All @@ -24,7 +25,13 @@ export const createExports = (manifest: SSRManifest, { middlewareSecret }: Args)
cacheOnDemandPages: boolean;
notFoundContent?: string;
}) {
// The entrypoint is created in `.netlify/v1/build` so we need to go up two levels to get to the root
const root = fileURLToPath(new URL('../../', import.meta.url));
return async function handler(request: Request, context: Context) {
// This entrypoint will be deep inside the directory structure below cwd
// We chdir to the root so that we can resolve the correct paths at runtime
process.chdir(root);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure it can't break any existing app?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can work out, existing apps that rely on cwd would either have unchanged behaviour (if they're not a monorepo), or would already be broken (if they are using a monorepo)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the "correct paths at runtime" refers to here. It feels dangerous to call chdir() as a request handler. Can you explain where is resolving from the cwd?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a site uses filesystem functions at runtime, this is the only way it can correctly load the files

Copy link
Member

@bluwy bluwy Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think using filesystem functions is something Astro supports in the runtime? Files could be moved during bundling and it wouldn't work later.

Is process.chdir() needed for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't prevent people using fs functions, so I'd imagine lots of people do. This PR was prompted by an issue where a library was trying to load i18n data and it wasn't found. This part fixes precisely that issue: of files being moved during bundling. Without that there's not a lot of point to allowing users to include files in the bundle, because the user code won't be able to find them.

Copy link
Member

@bluwy bluwy Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel like fs functions are a common thing especially when dealing in a bundled runtime. There's Vite APIs like import.meta.glob, ?url, and ?raw for them. Fs functions are also likely to fail elsewhere than only on Netlify, and so we shouldn't support and encourage them.

I figured the benefit with includeFiles and excludeFiles are like Vercel's option, because nft has trouble sometimes detecting every file. Especially inside node_modules where libraries use fs relatively, those code are not processed by Vite so we could use the option for them, however if they resolve relatively I'm not sure how cwd helps here. And if the library do resolve from cwd, it would've been generally a problem if an Astro is also hosted on a subdirectory or any other fine setup.

I'm not really comfortable with keeping this in the adapter. It feels surprising to me that it could change the cwd internally and on every request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are like Vercel's option. In fact they use almost the same implementation. With libraries that use fs functions, they usually accept a directory as an option or resolve relative to cwd. It's very common for i18n libraries such as i18next, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're about to do a major I can do a breaking changeset for the chdir part.


const routeData = app.match(request);
if (!routeData && typeof integrationConfig.notFoundContent !== 'undefined') {
return new Response(integrationConfig.notFoundContent, {
Expand Down
11 changes: 11 additions & 0 deletions packages/netlify/test/functions/fixtures/includes/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import netlify from '@astrojs/netlify';
import { defineConfig } from 'astro/config';

export default defineConfig({
output: 'server',
adapter: netlify({
includedFiles: ['files/**/*.csv', 'files/include-this.txt'],
excludedFiles: ['files/subdirectory/not-this.csv'],
}),
site: `http://example.com`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1,2,3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1,2,3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1,2,3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello
13 changes: 13 additions & 0 deletions packages/netlify/test/functions/fixtures/includes/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@test/netlify-includes",
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/netlify": "workspace:",
"astro": "^4.11.5"
},
"scripts": {
"build": "astro build",
"dev": "astro dev"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/// <reference types="astro/client" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
export const prerender = false
const header = Astro.request.headers.get("x-test")
---

<p>This is my custom 404 page</p>
<p>x-test: {header}</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
import { promises as fs } from 'fs';
const cwd = process.cwd();
const file = await fs.readFile('files/include-this.txt', 'utf-8');
---
<html>
<head><title>Testing</title></head>
<body>
<h1>{file}</h1>
<p>{cwd}</p>
</body>
</html>
54 changes: 54 additions & 0 deletions packages/netlify/test/functions/includes.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import * as assert from 'node:assert/strict';
import { existsSync } from 'node:fs';
import { before, describe, it } from 'node:test';
import { fileURLToPath } from 'node:url';
import { loadFixture } from '@astrojs/test-utils';
import * as cheerio from 'cheerio';

describe(
'Included files',
() => {
let fixture;
const root = new URL('./fixtures/includes/', import.meta.url);
const expectedCwd = new URL(
'.netlify/v1/functions/ssr/packages/netlify/test/functions/fixtures/includes/',
root
);

before(async () => {
fixture = await loadFixture({ root });
await fixture.build();
});

it('Includes files', async () => {
const filesRoot = new URL('./files/', expectedCwd);
const expectedFiles = ['include-this.txt', 'also-this.csv', 'subdirectory/and-this.csv'];

for (const file of expectedFiles) {
assert.ok(existsSync(new URL(file, filesRoot)), `Expected file ${file} to exist`);
}

const notExpectedFiles = ['subdirectory/not-this.csv', 'subdirectory/or-this.txt'];

for (const file of notExpectedFiles) {
assert.ok(!existsSync(new URL(file, filesRoot)), `Expected file ${file} to not exist`);
}
});

it('Can load included files correctly', async () => {
const entryURL = new URL(
'./fixtures/includes/.netlify/v1/functions/ssr/ssr.mjs',
import.meta.url
);
const { default: handler } = await import(entryURL);
const resp = await handler(new Request('http://example.com/'), {});
const html = await resp.text();
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'hello');
assert.equal($('p').text(), fileURLToPath(expectedCwd).slice(0, -1));
});
},
{
timeout: 120000,
}
);
Loading