Update netlify adapter to integrate includeFiles and excludeFiles options #515

dfdez · 2025-01-22T20:57:53Z

Changes

When trying Netlify SSR with Astro, I wasn't able to get the included_files option to work. After investigating, I realized that the includeFiles and excludeFiles functionalities, similar to what Vercel provides, were necessary to include additional files during the build process.

In this PR, I have implemented the integration to add these options:

Added includeFiles and excludeFiles options to the integration, mirroring Vercel's functionality.
- includeFiles: Allows users to explicitly specify files or directories to be bundled with their function. Useful for avoiding missing files in production.
- excludeFiles: Enables users to specify files or directories to be excluded from the deployment. Helps minimize bundle size.
All code for these features has been adapted directly from Vercel's integration for consistency and reliability.

Testing

Manually tested includeFiles and excludeFiles with various configurations to ensure:
- Files specified in includeFiles are bundled correctly.
- Files and directories specified in excludeFiles are omitted as expected.
Automated tests added to validate edge cases and typical usage scenarios.
Confirmed the implementation behaves identically to Vercel's integration.

Docs

Updated astro docs with documentation for includeFiles and excludeFiles on netlify adapter.
Added links to Vercel's documentation for further reference:
- includeFiles
- excludeFiles

changeset-bot · 2025-01-22T20:57:57Z

🦋 Changeset detected

Latest commit: 5557cc0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@astrojs/netlify	Minor
@test/netlify-hosted-astro-project	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

ematipico · 2025-01-23T06:28:27Z

Automated tests added to validate edge cases and typical usage scenarios.

It doesn't seem you added new assertions, and you didn't test the excludeFiles. Can you please do so?

ematipico

Can you please add a new changeset?

ascorbic · 2025-01-23T11:25:29Z

I have an old PR that does the same, but it also included changing the cwd at runtime, which in retropect should have been separate – and proved to be controversial – which is why I never merged it. Mine has now become outdated and conflicted, so having a new PR like this makes sense to me, but you might want to look at my PR to get some tests and changeset ideas.

dfdez · 2025-01-24T01:24:42Z

Thanks @ascorbic for the PR was very useful ✨

After taking a look to #325 I have made the following changes:

Added glob functionality based on feat(netlify): support included/excluded files #325
Added tests based on feat(netlify): support included/excluded files #325
Added changelogs copied from feat(netlify): support included/excluded files #325

packages/netlify/test/functions/fixtures/includes/src/pages/index.astro

ascorbic · 2025-01-24T09:12:39Z

.changeset/beige-insects-hang.md

+
+The `includedFiles` and `excludedFiles` options allow you specify these inclusions and exclusions as an array of file paths relative to the site root. Both options support glob patterns, so you can include/exclude multiple files at once.
+
+If you are specifying files using filesystem functions, resolve the path using `path.resolve()` or `process.cwd()`, which will give you the site root. At runtime, compiled source files will be in a different location and you cannot rely on relative file paths.


I don't think this will work in this PR, because it doesn't do chdir

I can't speak to the tech, but will note:

this feels a bit too detailed for a changelog

if someone needs an explanation like this to use the feature, it must be in docs itself

Changelogs update -> 5557cc0

ascorbic · 2025-01-24T09:14:13Z

packages/netlify/test/functions/fixtures/includes/astro.config.mjs

+
+export default defineConfig({
+  vite: {
+    assetsInclude: process.env.VITE_ASSETS_INCLUDE?.split(',') || [],


Rather than juggling env vars you can pass the actual config to loadFixture in each test

Done 👍🏽 -> 63ce3bb

ascorbic · 2025-01-24T09:14:44Z

packages/netlify/test/functions/include-files.test.js

+			process.env.VITE_ASSETS_INCLUDE = expectedAssetsInclude.join();
+			fixture = await loadFixture({ root });


Pass the config to loadFixture instead

sarah11918

Just some tiny comments on the changeset, and info that seems to be in this PR that did not make it into docs!

sarah11918 · 2025-01-24T12:53:47Z

.changeset/beige-insects-hang.md

+
+Adds `includedFiles` and `excludedFiles` configuration 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.


Suggested change

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.

When an Astro site using on-demand 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.

Noting that hybrid does not exist as a "code" word anymore (and, I'm not sure we use it casually?)

sarah11918 · 2025-01-24T12:56:02Z

.changeset/beige-insects-hang.md

+
+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 site root. Both options support glob patterns, so you can include/exclude multiple files at once.


I've made a note (in the Docs PR) that this content did not make it into the docs PR, but should!

sarah11918 · 2025-01-24T12:57:24Z

.changeset/beige-insects-hang.md

+
+The `includedFiles` and `excludedFiles` options allow you specify these inclusions and exclusions as an array of file paths relative to the site root. Both options support glob patterns, so you can include/exclude multiple files at once.
+
+If you are specifying files using filesystem functions, resolve the path using `path.resolve()` or `process.cwd()`, which will give you the site root. At runtime, compiled source files will be in a different location and you cannot rely on relative file paths.


I can't speak to the tech, but will note:

this feels a bit too detailed for a changelog

if someone needs an explanation like this to use the feature, it must be in docs itself

dfdez · 2025-01-30T22:45:41Z

@ascorbic I've made the requested changes to the tests, and as @sarah11918 requested, I've updated the changelog and made some updates to the docs as well. 👍🏽

Let me know if you need anything else! Thanks!

ematipico

I am not really confident about this feature because I don't know the code. I let @ascorbic doing th review

Not required anymore

sarah11918

Changeset is good! It could merge exactly like this, but I could even see adding a code example that shows both new properties configured (I'd maybe include one file, to show that it's still an array even if you only have one item, and then exclude two or three files, and/or a glob pattern?), but I'd usually make final recommendations when the feature is ready to be merged and the docs are done.

I can see that this feature still needs a code review/approval, so I won't super edit the corresponding docs just yet, but can confirm they're in good draft shape and I should have all that I need to whip those up if this feature is being accepted! (I do like to wait until it's pretty clear nothing more will change before doing a final polish to the documentation, just in case!)

So please ping me when this is close to being released, and I'll come in for a final swoop on both this and docs then! It won't take much! 🫡

ascorbic

Looks good! Once this is in, you might want to think about your proposal for exposing the fucntion root dir at runtime. If we were to do it, it would make sense to do the same for Vercel.

ematipico · 2025-02-07T14:30:43Z

Hi, we moved the netlify adapter in a new repository: https://github.com/withastro/astro/tree/main/packages/integrations/netlify

We will achieve this repository soon, so if you wish to land this PR, you'll have to create a new one.

ascorbic · 2025-02-07T14:33:02Z

@dfdez sorry we didn't get this in before the move. It should be fine to redo it as-is on the main repo, aside from the slightly changed base path

dfdez · 2025-02-07T16:21:33Z

No worries! I'll redo it on the main repo 👍🏽

feat: integrated includeFiles on netlify adapter

c7e38e6

github-actions bot added the pkg: netlify label Jan 22, 2025

dfdez changed the title ~~feat: integrated includeFiles on netlify adapter~~ feat: integrated includeFiles and excludeFiles on netlify adapter Jan 22, 2025

dfdez changed the title ~~feat: integrated includeFiles and excludeFiles on netlify adapter~~ Update netlify adapter to integrate includeFiles and excludeFiles options Jan 22, 2025

dfdez marked this pull request as ready for review January 22, 2025 21:29

dfdez mentioned this pull request Jan 22, 2025

Docs update for includeFiles and excludeFiles options on netlify adapter withastro/docs#10760

Merged

ematipico previously requested changes Jan 23, 2025

View reviewed changes

dfdez added 3 commits January 24, 2025 02:18

feat: integrate glob for include and exclude fileson netlify adapter

be0919f

feat: added tests

f6326b1

feat: changelogs added

8835be1

dfdez requested a review from ematipico January 24, 2025 01:24

dfdez commented Jan 24, 2025

View reviewed changes

packages/netlify/test/functions/fixtures/includes/src/pages/index.astro Show resolved Hide resolved

ascorbic reviewed Jan 24, 2025

View reviewed changes

sarah11918 reviewed Jan 24, 2025

View reviewed changes

dfdez added 2 commits January 30, 2025 23:18

feat: pass config on loadFixture instead of env

63ce3bb

feat: changelogs updated

5557cc0

dfdez requested review from sarah11918 and ascorbic January 30, 2025 22:45

ematipico reviewed Jan 31, 2025

View reviewed changes

sarah11918 reviewed Feb 3, 2025

View reviewed changes

ascorbic approved these changes Feb 3, 2025

View reviewed changes

ematipico closed this Feb 7, 2025

dfdez mentioned this pull request Feb 7, 2025

Update netlify adapter to integrate includeFiles and excludeFiles options withastro/astro#13194

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update netlify adapter to integrate includeFiles and excludeFiles options #515

Update netlify adapter to integrate includeFiles and excludeFiles options #515

dfdez commented Jan 22, 2025 •

edited

Loading

changeset-bot bot commented Jan 22, 2025 •

edited

Loading

ematipico commented Jan 23, 2025

ematipico left a comment

ascorbic commented Jan 23, 2025

dfdez commented Jan 24, 2025

ascorbic Jan 24, 2025

sarah11918 Jan 24, 2025

dfdez Jan 30, 2025

ascorbic Jan 24, 2025

dfdez Jan 30, 2025

ascorbic Jan 24, 2025

sarah11918 left a comment

sarah11918 Jan 24, 2025

sarah11918 Jan 24, 2025

sarah11918 Jan 24, 2025

dfdez commented Jan 30, 2025

ematipico left a comment

sarah11918 left a comment

ascorbic left a comment

ematipico commented Feb 7, 2025

ascorbic commented Feb 7, 2025

dfdez commented Feb 7, 2025


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

		If you are specifying files using filesystem functions, resolve the path using `path.resolve()` or `process.cwd()`, which will give you the site root. At runtime, compiled source files will be in a different location and you cannot rely on relative file paths.

		process.env.VITE_ASSETS_INCLUDE = expectedAssetsInclude.join();
		fixture = await loadFixture({ root });


		Adds `includedFiles` and `excludedFiles` configuration 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.

Update netlify adapter to integrate includeFiles and excludeFiles options #515

Update netlify adapter to integrate includeFiles and excludeFiles options #515

Conversation

dfdez commented Jan 22, 2025 • edited Loading

Changes

Testing

Docs

changeset-bot bot commented Jan 22, 2025 • edited Loading

🦋 Changeset detected

ematipico commented Jan 23, 2025

ematipico left a comment

Choose a reason for hiding this comment

ascorbic commented Jan 23, 2025

dfdez commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarah11918 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfdez commented Jan 30, 2025

ematipico left a comment

Choose a reason for hiding this comment

sarah11918 left a comment

Choose a reason for hiding this comment

ascorbic left a comment

Choose a reason for hiding this comment

ematipico commented Feb 7, 2025

ascorbic commented Feb 7, 2025

dfdez commented Feb 7, 2025

dfdez commented Jan 22, 2025 •

edited

Loading

changeset-bot bot commented Jan 22, 2025 •

edited

Loading