Update netlify adapter to integrate includeFiles and excludeFiles options by dfdez · Pull Request #13194 · withastro/astro

dfdez · 2025-02-07T18:14:55Z

This PR implements the changes previously proposed in withastro/adapters#515, migrated to work within this repository's structure. The core functionality remains the same, with adjustments made to align with this repository.

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-02-07T18:15:01Z

🦋 Changeset detected

Latest commit: 194a27b

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

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

Thank you! I left some questions, and also we should make the docs a bit more explicit because there's some important information that we should expose to users.

ematipico · 2025-02-10T14:07:05Z

+			if (_config.vite.assetsInclude) {
+				const mergeGlobbedIncludes = (globPattern: unknown) => {
+					if (typeof globPattern === 'string') {
+						const entries = glob.sync(globPattern).map((p) => pathToFileURL(p));


Two questions:

Why aren't these assets resolved from the same cwd of line 263?

Why don't we ignore exclude?

Why aren't these assets resolved from the same cwd of line 263?

That is a good question; this code were taken from the vercel integration not sure why that difference does currently exists...

Maybe it does have sense to update this and use the getFilesByGlob function to have the same behavior on either includeFiles and vite.assetsInclude.

Why don't we ignore exclude?

Similar to previous response; this code were taken from the vercel integration but I think there isn't any specific reason to don't exclude the files therefore we can add it.

I just wanted to keep the most consistency between integration, for those reasons I decided to keep this code as is. Let me know if any modifications are needed, and I'll gladly update it 👌

It looks like the code is been there for a long time, let's keep it as is. Thank you for taking time for investigating and providing me with a proper response!

While the Vercel code has been there for a long time, this is new code for Netlify, so I think it would make sense to make it consistent with the cwd at least

ematipico · 2025-02-10T14:07:40Z

+	includeFiles?: string[];
+
+	/** Exclude any files from the bundling process that would otherwise be included. */
+	excludeFiles?: string[];


There's an important piece of information that is missing from the inline JS docs. The code resolves the files from the rootDir. This information must be shared with the docs. This information is also missing from the docs PR (I checked).

What does that mean? If a user adds an absolute path, this might fail. We should provide this information.

I've added a note on the JS Docs 93c2acf#diff-c395613459736f27ff2bafd942481c8713261bd79538e1234994f9d3d4d0be5bR148

Also I will make sure to update the docs to add this important information 👍

Can you add it to the excludeFiles block too

ematipico

Looks good to me!

ematipico · 2025-02-11T11:58:50Z

+			if (_config.vite.assetsInclude) {
+				const mergeGlobbedIncludes = (globPattern: unknown) => {
+					if (typeof globPattern === 'string') {
+						const entries = glob.sync(globPattern).map((p) => pathToFileURL(p));


It looks like the code is been there for a long time, let's keep it as is. Thank you for taking time for investigating and providing me with a proper response!

sarah11918

Just left an idea of a sample changeset, for your consideration!

Co-authored-by: Matt Kane <m@mk.gg>

Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>

dfdez added 2 commits February 7, 2025 19:06

feat: integrated includeFiles and excludeFiles on netlify adapter

d466ab8

feat: added netlify include and exclude files tests

46b6b25

github-actions Bot added the pkg: integration Related to any renderer integration (scope) label Feb 7, 2025

feat: changelogs added

018d341

dfdez mentioned this pull request Feb 7, 2025

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

Merged

fix: avoid problems with glob file url on windows

a1bcac7

dfdez force-pushed the feat/netlify-include-files branch from 7d6936e to a1bcac7 Compare February 8, 2025 16:48

ematipico reviewed Feb 10, 2025

View reviewed changes

dfdez added 2 commits February 10, 2025 23:10

feat: improved JS Docs to include examples and important information

93c2acf

feat: removed non necessary root path as glob is already absolute

46a9895

ematipico approved these changes Feb 11, 2025

View reviewed changes

ematipico added this to the 5.3.0 milestone Feb 11, 2025

ascorbic reviewed Feb 11, 2025

View reviewed changes

Comment thread packages/integrations/netlify/src/index.ts Outdated

Apply suggestions from code review

6302bb6

sarah11918 reviewed Feb 11, 2025

View reviewed changes

Comment thread .changeset/ninety-clouds-judge.md Outdated

ematipico and others added 2 commits February 12, 2025 13:16

Update packages/integrations/netlify/src/index.ts

796d697

Co-authored-by: Matt Kane <m@mk.gg>

Update .changeset/ninety-clouds-judge.md

194a27b

Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>

ascorbic approved these changes Feb 12, 2025

View reviewed changes

ematipico merged commit 1b5037b into withastro:main Feb 12, 2025

This was referenced Feb 12, 2025

[ci] release #13230

Merged

[ci] release #13244

Merged

Uh oh!

Conversation

dfdez commented Feb 7, 2025

Changes

Testing

Docs

Uh oh!

changeset-bot Bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

ematipico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ematipico Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

dfdez Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

ematipico Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

ascorbic Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

ematipico Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfdez Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

ascorbic Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

ematipico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ematipico Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sarah11918 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

changeset-bot Bot commented Feb 7, 2025 •

edited

Loading

ematipico Feb 10, 2025 •

edited

Loading