Conversation
- Enable previously skipped prerender tests for both serverless and edge sites. - The tests were disabled due to React Router issue #13226, but now work correctly after the rollupOptions.input merge fix in plugin.ts.
- Extract a function that thoroughly handles the rollup input merging complexity, with its own coverage - Simplify the plugin's `config` hook to leverage the hook's "just return a deep partial update object" behaviour instead of mutating the passed-in config
✅ Deploy Preview for remix-edge ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for remix-serverless ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
See 9f930c9. We bumped netlify-cli without noticing that it changed the supported deno version.
| if (typeof input === 'string') { | ||
| // For a single string input, the default entry name is 'index' | ||
| return { index: input } | ||
| } |
There was a problem hiding this comment.
I tried vanilla rollup project with config like so:
export default {
input: "src/main.js",
output: {
entryFileNames: "[name].js",
format: "cjs",
},
};and using npx rollup -c -d dist result in dist/main.js being produced, not dist/index.js
... but maybe vite (or even react-router) does something here and not just rollup 🤷
But also this might be path that is never actually used in practice in react router projects, so not sure if it matters if this is not fully right 🤷
There was a problem hiding this comment.
Great catch. You're absolutely right. I was misled by files that happened to end with /index.js 🤦🏼. Fixed in bd3f0ec.
pieh
left a comment
pieh
left a comment
There was a problem hiding this comment.
Despite the one question - all our tests (including unskipped one) is passing, so this looks OK to me 👍
serhalp
deleted the
serhalp/frb-2082-bug-rollupoptionsinput-overwrite-breaks-react-router
branch
4 participants
Description
@netlify/vite-plugin-react-routerwas overwriting the entireconfig.build.rollupOptions.inputvalue during SSR build, which removed entries set by other plugins. This was intentional, because we believed RR7's original SSR bundle entry wasn't needed (our entry for our generated Netlify Function references it). It turns out that when opting in to React Router 7 framework mode prerendering, this bundle output is required. Some details can be found here: remix-run/react-router#13226 (comment).This PR fixes the issue by preserving existing input entries and merging them with Netlify's entry.
The repo already skipped e2e tests for prendering. This PR unskips them and they now just work 🎉.
Related Tickets & Documents
rollupOptions.inputoverwrite breaks React Router prerender #606rollupOptions.inputoverwrite breaks React Router prerender #606 (thank you @nahyeongjin1!)QA Instructions, Screenshots, Recordings
Reproduction Steps
npx create-react-router@latest --template netlify/react-router-template cd ./your-project npm install npx netlify initreact-router.config.tswith:npx netlify deploy # or even just `npm run build`Before this fix
Build fails with:
After this fix
Build succeeds with prerender working: