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

fix: rework browser optimizeDeps.entries #560

Closed
wants to merge 1 commit into from

Conversation

hi-ogawa
Copy link
Owner

@hi-ogawa hi-ogawa commented Jul 14, 2024

I thought having this is safe, but in practice, it looks like this would cause pre-bundling error (or just warning?) since it goes through a lot of server only dependencies whose resolutions somehow fail on browser resolver.

One example is when crawling @node-rs/argon2 hi-ogawa/lucia-auth-examples#1 (comment)

Not having this is okay since this is only to prevent dev full reload on late dep discovery (also users still have a way to configure it on their own), but there might be still a cheap attempt to mitigate this.


Some ideas:

  • remove automatic optimizeDeps.entries and let users configure it (with optimizeDeps.exclude to counter against if they need)
  • custom esbuild plugin to crawl only "use client" files?
    • we don't probably have to go deep and we can only check direct dependency of all routes/**/* files
  • can we push to optimizeDeps.entries after we collected clientReferenceMap by loading server routes

Copy link
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @hi-ogawa and the rest of your teammates on Graphite Graphite

@hi-ogawa hi-ogawa force-pushed the fix-no-aggressive-browser-optimizeDeps branch from 313230a to f0503e9 Compare July 15, 2024 07:46
Copy link

pkg-pr-new bot commented Jul 15, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

commit: d4ea790

@hiogawa/react-server

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/react-server@560

@hiogawa/react-server-next

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/react-server-next@560

@hiogawa/transforms

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/transforms@560

@hiogawa/vite-plugin-ssr-middleware

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/vite-plugin-ssr-middleware@560


templates

@hi-ogawa hi-ogawa force-pushed the fix-no-aggressive-browser-optimizeDeps branch from f0503e9 to d4ea790 Compare July 15, 2024 07:58
@hi-ogawa
Copy link
Owner Author

Hmm, it doesn't look really possible to precisely detect dependencies like @node-rs/argon2. Probably hard-coding them like Next.js https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/server-external-packages.json is the best bet... (cf. #545)

Closing for now.

@hi-ogawa hi-ogawa closed this Jul 15, 2024
@hi-ogawa hi-ogawa deleted the fix-no-aggressive-browser-optimizeDeps branch July 15, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rework browser optimizeDeps
1 participant