Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve optimizeDeps.entries to avoid server endpoints #10143

Merged
merged 6 commits into from
Feb 22, 2024
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Feb 16, 2024

Changes

close #9859

Previously, optimizeDeps.entries set does not respect the srcDir option and globs into server endpoints, which we know should not contain browser code. optimizeDeps should only be used on browser code.

This PR fixes by respecting srcDir, and make sure server endpoints are not included in the glob. While this is not perfect in the bigger picture, and the ideal solution is to add a custom esbuild plugin to transform Astro files to allow the scanner to scan the appropriate browser files, this PR is an easier and quicker fix to the issue for now.

Testing

It's a bit hard to test the overall behaviour, but I ran DEBUG="vite:deps" pnpm dev in the example projects to check that it logs the correct files globbed by optimizeDeps.entries.

Docs

n/a. bug fix.

Copy link

changeset-bot bot commented Feb 16, 2024

🦋 Changeset detected

Latest commit: f7224d6

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Feb 16, 2024
entries: ['src/**/*'],
// Scan all files within `srcDir` except for known server-code (e.g endpoints)
entries: [
`${srcDirPattern}!(pages)/**/*`, // All files except for pages
Copy link
Member Author

Choose a reason for hiding this comment

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

I made some tests and it seems that srcDirPattern will always have a trailing slash. The only case I'm not sure is on Windows.

// Scan all files within `srcDir` except for known server-code (e.g endpoints)
entries: [
`${srcDirPattern}!(pages)/**/*`, // All files except for pages
`${srcDirPattern}pages/**/!(*.js|*.ts)`, // All pages except for endpoints
Copy link
Member

Choose a reason for hiding this comment

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

Ehehe, unfortunately mjs and cjs should be included, even though it's a low chance that users use them. Still, we should

Copy link
Member Author

@bluwy bluwy Feb 21, 2024

Choose a reason for hiding this comment

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

Good catch! I'll add this too. I think perhaps for mjs and mts only. cjs and cts is unlikely to work in Astro and should have errors elsewhere if used.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Looks good!

@bluwy bluwy merged commit 7c5fcd2 into main Feb 22, 2024
13 checks passed
@bluwy bluwy deleted the no-optimize-endpoint branch February 22, 2024 10:36
@astrobot-houston astrobot-houston mentioned this pull request Feb 22, 2024
log101 pushed a commit to log101/astro that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude server endpoints from Vite's dependency optimization
2 participants