feat: support Vite Env API and React Router 8 - #714
Conversation
✅ 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR fixes React Router v8 compatibility by rewriting Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/vite-plugin-react-router/src/plugin.tsOops! Something went wrong! :( ESLint: 8.57.1 YAMLException: Cannot read config file: /.eslintrc.mjs 1 | import { overrides } from ' Comment |
github-actions
Bot
added
the
type: feature
React Router 8 makes the Vite Environment API mandatory and supports Vite 7-8. RR7 supports Vite 6-7
but users can opt in to this mode as well via `future.v8_viteEnvironmentApi`.
Either way, the Vite Env API is forward compatible from the "client" and "ssr" pseudo-environments
present previously, and those are backwards compatible as well.
The main change to contend with here is reading from `config.environments?.ssr?.build` instead of
`config.ssr.build` directly. This is supported by all combinations of versions and flags listed
above.
We also replace `isSsrBuild` with a simple `applyToEnvironment` hook, which is compatible with all
these combinations as well.
Finally, this starts reading from `rolldownOptions` and falling back to `rollupOptions`. This was
entirely optional, but since the latter is soft-deprecated, I introduced this here. Again, all
combinations of versions and flags are compatible with this.
```
Vite 5 Vite 6 Vite 7 Vite 8
RR7 EOL ✓ ✓ ✓ (Vite 8 requires RR 7.14.0+)
RR8 EOL — ✓ ✓
```
It seems setting `resolve.conditions` is just a subset of what `ssr.target: "webworker"` does, so we still need this.
4114f7d to
026ae77
Compare
The previous approach was to register our own input that wraps the (leakily) expected React Router input. This is now fundamentally incompatible with React Router 8 as soon as a site has any prerendered pages, because React Router 8 uses a separate Vite Environment `prerender` that it builds by first building the other envs, then starting a preview server, making real HTTP requests to each path to be prerendered, and saving the response to disk. This request was failing due to some assumption in React Router 8 that is not happy with its own entry coexisting with another: https://github.com/remix-run/react-router/blob/da102b5928d2cf0b94cd0abc2895d0939698e7e9/packages/react-router-dev/vite/plugin.ts#L3578-L3586
026ae77 to
ef0b876
Compare
| jobs: | ||
| e2e: | ||
| runs-on: ubuntu-latest | ||
| runs-on: ubuntu-24.04-arm |
There was a problem hiding this comment.
Tends to be ~30% faster, which is quite helpful for these ~260 e2e tests
| /** | ||
| * Find React Router's single built server entry chunk and return its on-disk path. | ||
| * This assumes that there is exactly one, which we happen to know to be true... except | ||
| * when the user uses the `serverBundles` feature, which we do not support. |
There was a problem hiding this comment.
To be clear, we already didn't support this
| return { | ||
| name: 'vite-plugin-netlify-react-router', | ||
| config(_config, { command, isSsrBuild }) { | ||
| applyToEnvironment: (environment) => environment.name === 'ssr', |
There was a problem hiding this comment.
I would have liked to use environment.consumer === 'server' here, but then this runs for RR's prerender environment as well.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef0b876a6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { type Fixture } from './support/deploy-to-netlify' | ||
| import { expect, test } from './support/fixtures' |
There was a problem hiding this comment.
self-nit: re-export this from ./support/fixtures
| import { type Fixture } from './support/deploy-to-netlify' | |
| import { expect, test } from './support/fixtures' | |
| import { expect, test, type Fixture } from './support/fixtures' |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e.yml (1)
1-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a permissions block to limit GITHUB_TOKEN scope.
The workflow lacks an explicit permissions declaration for the
GITHUB_TOKEN, which is a security best practice. This allows the job to use all available GitHub API permissions by default.🔐 Proposed fix to add a minimal permissions block
e2e: + permissions: + contents: read runs-on: ubuntu-24.04-armAdjust the
contents: readscope as needed based on what this workflow actually requires (e.g., if it pushes artifacts or comments, expand accordingly).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e.yml around lines 1 - 56, Add an explicit permissions block to the e2e workflow to limit the scope of the GITHUB_TOKEN for security purposes. Insert a permissions section at the job level (under the e2e job, before the runs-on declaration) or at the workflow level (after the on section). Set the permissions to the minimum required by the workflow, starting with contents: read, and expand the scope only if the workflow requires additional permissions such as pushing artifacts, writing comments, or other GitHub API operations.Source: Linters/SAST tools
🧹 Nitpick comments (1)
packages/vite-plugin-react-router/src/plugin.test.ts (1)
6-9: ⚡ Quick winStrengthen this test to validate plugin contract, not just object-ness.
toBeInstanceOf(Object)is too broad and won’t catch regressions in required hooks introduced by this rewrite. Assert the key plugin fields/hooks explicitly.Proposed test improvement
import { expect, it } from 'vitest' -import { Plugin } from 'vite' import { netlifyPlugin } from './plugin.js' it('exports a Vite plugin factory', () => { const plugin = netlifyPlugin() - expect(plugin).toBeInstanceOf(Object) + expect(plugin).toMatchObject({ + name: 'vite-plugin-netlify-react-router', + }) + expect(plugin.applyToEnvironment).toBeTypeOf('function') + expect(plugin.writeBundle).toBeTypeOf('function') })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vite-plugin-react-router/src/plugin.test.ts` around lines 6 - 9, The test for netlifyPlugin() in the 'exports a Vite plugin factory' test case is too generic by only asserting that the result is an instance of Object. Replace this broad check with explicit assertions that validate the plugin implements the required Vite plugin contract by checking for the specific hooks and fields expected from a Vite plugin (such as name, apply, and other required plugin methods/properties). This will catch regressions if the plugin structure changes and doesn't include essential Vite plugin hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/fixtures/react-router-edge-site-v8/app/app.css`:
- Around line 1-3: The `@tailwind` directives in the app.css file are triggering
the scss/at-rule-no-unknown stylelint rule. Add a local stylelint suppression
comment above the three `@tailwind` at-rules (base, components, and utilities) to
suppress this specific rule for these lines, allowing the fixture to use
Tailwind's standard directives without lint errors flagging them.
In `@tests/e2e/fixtures/react-router-serverless-site-v8/app/app.css`:
- Around line 1-3: The `@tailwind` directives in app.css are being flagged as
unknown at-rules by Stylelint. Fix this by updating the Stylelint configuration
file to recognize Tailwind at-rules by adding a rule that allows them (such as
configuring allowedAtRules or installing a Tailwind-specific Stylelint plugin).
If modifying the global config is not preferred, alternatively add a local /*
stylelint-disable-line at-rule-no-unknown */ comment above each of the three
`@tailwind` directives (base, components, utilities) in the CSS file to suppress
the warning for this fixture.
In `@tests/e2e/react-router-user-journeys.spec.ts`:
- Around line 238-309: The test suite is missing coverage for the React Router 7
with Vite 8 combination. Add two new test.describe blocks following the existing
pattern: one for 'origin SSR (React Router 7, Vite 8)' that calls
defineOriginSsrTests with the reactRouterServerlessSiteRR7Vite8 fixture (insert
between the RR7 Vite 7 and RR8 Vite 7 origin SSR test blocks), and another for
'edge SSR (React Router 7, Vite 8)' that calls defineEdgeSsrTests with the
reactRouterEdgeSiteRR7Vite8 fixture (insert between the RR7 Vite 7 and RR8 Vite
7 edge SSR test blocks). Each block should follow the same pattern as the
surrounding test suites by wrapping the fixture parameter passed to the callback
function.
---
Outside diff comments:
In @.github/workflows/e2e.yml:
- Around line 1-56: Add an explicit permissions block to the e2e workflow to
limit the scope of the GITHUB_TOKEN for security purposes. Insert a permissions
section at the job level (under the e2e job, before the runs-on declaration) or
at the workflow level (after the on section). Set the permissions to the minimum
required by the workflow, starting with contents: read, and expand the scope
only if the workflow requires additional permissions such as pushing artifacts,
writing comments, or other GitHub API operations.
---
Nitpick comments:
In `@packages/vite-plugin-react-router/src/plugin.test.ts`:
- Around line 6-9: The test for netlifyPlugin() in the 'exports a Vite plugin
factory' test case is too generic by only asserting that the result is an
instance of Object. Replace this broad check with explicit assertions that
validate the plugin implements the required Vite plugin contract by checking for
the specific hooks and fields expected from a Vite plugin (such as name, apply,
and other required plugin methods/properties). This will catch regressions if
the plugin structure changes and doesn't include essential Vite plugin hooks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7edbbde-23fa-403f-88e6-cd2873c4fa83
⛔ Files ignored due to path filters (8)
tests/e2e/fixtures/react-router-edge-site-v8/app/welcome/logo-dark.svgis excluded by!**/*.svgtests/e2e/fixtures/react-router-edge-site-v8/app/welcome/logo-light.svgis excluded by!**/*.svgtests/e2e/fixtures/react-router-edge-site-v8/public/camel.jpgis excluded by!**/*.jpgtests/e2e/fixtures/react-router-edge-site-v8/public/favicon.icois excluded by!**/*.icotests/e2e/fixtures/react-router-serverless-site-v8/app/welcome/logo-dark.svgis excluded by!**/*.svgtests/e2e/fixtures/react-router-serverless-site-v8/app/welcome/logo-light.svgis excluded by!**/*.svgtests/e2e/fixtures/react-router-serverless-site-v8/public/camel.jpgis excluded by!**/*.jpgtests/e2e/fixtures/react-router-serverless-site-v8/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (53)
.github/workflows/e2e.ymlpackages/vite-plugin-react-router/src/lib/rollup.test.tspackages/vite-plugin-react-router/src/lib/rollup.tspackages/vite-plugin-react-router/src/plugin.test.tspackages/vite-plugin-react-router/src/plugin.tstests/e2e/fixtures/react-router-edge-site-v8/app/app.csstests/e2e/fixtures/react-router-edge-site-v8/app/entry.server.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/root.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes.tstests/e2e/fixtures/react-router-edge-site-v8/app/routes/about.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/blobs.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/cacheable.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/context.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/headers.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/home.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/images.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/middleware-header.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/routes/prerendered.tsxtests/e2e/fixtures/react-router-edge-site-v8/app/welcome/welcome.tsxtests/e2e/fixtures/react-router-edge-site-v8/netlify.tomltests/e2e/fixtures/react-router-edge-site-v8/netlify/edge-functions/middleware-header.tstests/e2e/fixtures/react-router-edge-site-v8/netlify/edge-functions/pong.tstests/e2e/fixtures/react-router-edge-site-v8/netlify/functions/blorble.tstests/e2e/fixtures/react-router-edge-site-v8/package.jsontests/e2e/fixtures/react-router-edge-site-v8/react-router.config.tstests/e2e/fixtures/react-router-edge-site-v8/tailwind.config.tstests/e2e/fixtures/react-router-edge-site-v8/tsconfig.jsontests/e2e/fixtures/react-router-edge-site-v8/vite.config.tstests/e2e/fixtures/react-router-serverless-site-v8/app/app.csstests/e2e/fixtures/react-router-serverless-site-v8/app/root.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes.tstests/e2e/fixtures/react-router-serverless-site-v8/app/routes/about.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/blobs.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/cacheable.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/context.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/headers.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/home.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/images.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/middleware-header.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/routes/prerendered.tsxtests/e2e/fixtures/react-router-serverless-site-v8/app/welcome/welcome.tsxtests/e2e/fixtures/react-router-serverless-site-v8/netlify.tomltests/e2e/fixtures/react-router-serverless-site-v8/netlify/edge-functions/middleware-header.tstests/e2e/fixtures/react-router-serverless-site-v8/netlify/edge-functions/pong.tstests/e2e/fixtures/react-router-serverless-site-v8/netlify/functions/blorble.tstests/e2e/fixtures/react-router-serverless-site-v8/package.jsontests/e2e/fixtures/react-router-serverless-site-v8/react-router.config.tstests/e2e/fixtures/react-router-serverless-site-v8/tailwind.config.tstests/e2e/fixtures/react-router-serverless-site-v8/tsconfig.jsontests/e2e/fixtures/react-router-serverless-site-v8/vite.config.tstests/e2e/react-router-user-journeys.spec.tstests/e2e/support/deploy-to-netlify.tstests/e2e/support/fixtures.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
💤 Files with no reviewable changes (2)
- packages/vite-plugin-react-router/src/lib/rollup.test.ts
- packages/vite-plugin-react-router/src/lib/rollup.ts
🤖 I have created a release *beep* *boop* --- <details><summary>vite-plugin-react-router: 4.0.0</summary> ## [4.0.0](vite-plugin-react-router-v3.1.1...vite-plugin-react-router-v4.0.0) (2026-06-18) ### ⚠ BREAKING CHANGES * publish ESM only, remove CJS build ([#713](#713)) * require vite 6+, node.js 22.22+ ([#712](#712)) ### Features * publish ESM only, remove CJS build ([#713](#713)) ([1dece2c](1dece2c)) * require vite 6+, node.js 22.22+ ([#712](#712)) ([a99b5c4](a99b5c4)) * support Vite Env API and React Router 8 ([#714](#714)) ([eeb3a82](eeb3a82)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
2 participants
Description
React Router 8 has just been released.
React Router 8 makes the Vite Environment API mandatory and supports Vite 7-8. React Router 7 supports Vite 6-7 (and 8, starting with 7.14) but users can opt in to this mode as well via
future.v8_viteEnvironmentApi.Either way, the Vite Env API is forward compatible from the "client" and "ssr" pseudo-environments present previously, and those are backwards compatible as well.
The main change to contend with here is reading from
config.environments?.ssr?.buildinstead ofconfig.ssr.builddirectly. This is supported by all combinations of versions and flags listed above. Edit: I ended up doing this via theconfigEnvironmenthook, which is equivalent but runs after React Router has actually configured its environments.We also replace
isSsrBuildwith a simpleapplyToEnvironmenthook, which is compatible with all these combinations as well.Finally, this starts reading from
rolldownOptionsand falling back torollupOptions. This was entirely optional, but since the latter is soft-deprecated, I introduced this here. Again, all combinations of versions and flags are compatible with this.Essentially:
Now, unfortunately I ended up also needing to somewhat fundamentally rework the whole plugin's approach. The previous approach was to register our own input that wraps the (leakily) expected React Router input. This is now fundamentally incompatible with React Router 8 as soon as a site has any prerendered pages, because React Router 8 uses a separate Vite Environment
prerenderthat it builds by first building the other envs, then starting a preview server, making real HTTP requests to each path to be prerendered, and saving the response to disk. This request was failing, seemingly due to someassumption in React Router 8 that is not happy with its own entry coexisting with another. Funny enough, I more or less ended up just porting parts of our framework-agnostic plugin here.
The bulk of the diff in this PR is adding two new fixtures for RR8 serverless + RR7 edge, and refactoring the existing test suite to allow running table tests with different versions of Vite. This was important because in testing this locally I contended with all sorts of different behaviours across combinations. This is a bit clunky and will slow down CI but I believe it is worthwhile given all this combinatorial complexity.
Closes #698
Related Tickets & Documents
See also #711 and #712