Skip to content

Commit 54851a9

Browse files
fix(vite-plugin-react-router): fix prerendering by not clobbering SSR bundle entries (#620)
* test: enable prerender e2e tests after rollupOptions.input fix - 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. * fix: generalize and harden rollup input merging fix - 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 * test: remove placeholder test * fix: avoid importing transitive rollup dep * ci: install supported deno version See 9f930c9. We bumped netlify-cli without noticing that it changed the supported deno version. * fix: use rollup's exact bundle entry default name logic * fix: mimic correct behaviour for string input --------- Co-authored-by: 나형진 <skgudwls@konkuk.ac.kr>
1 parent fde82a6 commit 54851a9

File tree

12 files changed

+190
-53
lines changed

12 files changed

+190
-53
lines changed

.github/workflows/e2e.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ jobs:
2727
- name: Install Deno
2828
uses: denoland/setup-deno@v2
2929
with:
30-
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
31-
deno-version: v2.2.4
30+
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/8ff162649f76a9586dd7593198c0fa798dac3629/packages/edge-bundler/node/bridge.ts#L22
31+
deno-version: v2.4.2
3232
- run: pnpm install
3333

3434
- name: Install Playwright Browsers

.github/workflows/publint.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ jobs:
2727
- name: Install Deno
2828
uses: denoland/setup-deno@v2
2929
with:
30-
# Should satisfy the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
31-
deno-version: v2.2.4
30+
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/8ff162649f76a9586dd7593198c0fa798dac3629/packages/edge-bundler/node/bridge.ts#L22
31+
deno-version: v2.4.2
3232
- name: Install dependencies
3333
run: pnpm install
3434
- name: Build

.github/workflows/release-please.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ jobs:
3030
- name: Install Deno
3131
uses: denoland/setup-deno@v2
3232
with:
33-
# Should satisfy the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
34-
deno-version: v2.2.4
33+
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/8ff162649f76a9586dd7593198c0fa798dac3629/packages/edge-bundler/node/bridge.ts#L22
34+
deno-version: v2.4.2
3535
if: ${{ steps.release.outputs.releases_created }}
3636
- run: corepack enable
3737
- name: Install dependencies

.github/workflows/test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ jobs:
2424
- name: Install Deno
2525
uses: denoland/setup-deno@v2
2626
with:
27-
# Should satisfy the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
28-
deno-version: v2.2.4
27+
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/8ff162649f76a9586dd7593198c0fa798dac3629/packages/edge-bundler/node/bridge.ts#L22
28+
deno-version: v2.4.2
2929
- name: Install
3030
run: pnpm install
3131
- name: Build
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, it, expect } from 'vitest'
2+
3+
import { normalizeRollupInput, mergeRollupInput } from './rollup.js'
4+
5+
describe('normalizeRollupInput', () => {
6+
it('returns empty object for undefined input', () => {
7+
expect(normalizeRollupInput(undefined)).toEqual({})
8+
})
9+
10+
it('uses filename (without extension) as the entry name for string input', () => {
11+
expect(normalizeRollupInput('src/main.ts')).toEqual({
12+
main: 'src/main.ts',
13+
})
14+
})
15+
16+
it('uses full string as entry name when there is no path separator', () => {
17+
// basename('virtual:some-module') returns 'virtual:some-module' since there's no /
18+
expect(normalizeRollupInput('virtual:some-module')).toEqual({
19+
'virtual:some-module': 'virtual:some-module',
20+
})
21+
})
22+
23+
describe('array input', () => {
24+
it('returns empty object for empty array', () => {
25+
expect(normalizeRollupInput([])).toEqual({})
26+
})
27+
28+
it('uses filename as entry name for each entry', () => {
29+
expect(normalizeRollupInput(['src/main.ts', 'src/worker.ts'])).toEqual({
30+
main: 'src/main.ts',
31+
worker: 'src/worker.ts',
32+
})
33+
})
34+
35+
it('treats virtual modules the same as filenames', () => {
36+
expect(normalizeRollupInput(['virtual:react-router/server-build'])).toEqual({
37+
'server-build': 'virtual:react-router/server-build',
38+
})
39+
})
40+
})
41+
42+
describe('object input', () => {
43+
it('returns empty object for empty object input', () => {
44+
expect(normalizeRollupInput({})).toEqual({})
45+
})
46+
47+
it('returns the object as is', () => {
48+
const input = { main: 'src/main.ts', other: 'src/other.ts' }
49+
expect(normalizeRollupInput(input)).toEqual(input)
50+
})
51+
})
52+
})
53+
54+
describe('mergeRollupInput', () => {
55+
it('returns new entries when existing is undefined', () => {
56+
expect(mergeRollupInput(undefined, { server: 'virtual:netlify-server' })).toEqual({
57+
server: 'virtual:netlify-server',
58+
})
59+
})
60+
61+
it('merges with string existing input', () => {
62+
expect(mergeRollupInput('src/main.ts', { server: 'virtual:netlify-server' })).toEqual({
63+
main: 'src/main.ts',
64+
server: 'virtual:netlify-server',
65+
})
66+
})
67+
68+
it('merges with array existing input', () => {
69+
expect(mergeRollupInput(['src/main.ts', 'src/worker.ts'], { server: 'virtual:netlify-server' })).toEqual({
70+
main: 'src/main.ts',
71+
worker: 'src/worker.ts',
72+
server: 'virtual:netlify-server',
73+
})
74+
})
75+
76+
it('merges with object existing input', () => {
77+
expect(mergeRollupInput({ app: 'src/main.ts' }, { server: 'virtual:netlify-server' })).toEqual({
78+
app: 'src/main.ts',
79+
server: 'virtual:netlify-server',
80+
})
81+
})
82+
83+
it('allows new entries to override existing keys', () => {
84+
expect(mergeRollupInput({ server: 'old-server.ts' }, { server: 'virtual:netlify-server' })).toEqual({
85+
server: 'virtual:netlify-server',
86+
})
87+
})
88+
})
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { basename, extname } from 'node:path'
2+
3+
import type { Rollup } from 'vite'
4+
5+
/**
6+
* Derives the default entry name from a module ID, matching Rollup's internal behavior.
7+
* Unfortunately this logic isn't exposed publicly, but it's fairly simple (foo/bar/baz.ts -> baz)
8+
* so we replicate it here.
9+
* @see https://github.com/rollup/rollup/blob/fed6c1d/src/utils/relativeId.ts#L4-L7
10+
*/
11+
const getAliasName = (id: string): string => {
12+
const base = basename(id)
13+
return base.slice(0, Math.max(0, base.length - extname(id).length))
14+
}
15+
16+
/**
17+
* Normalizes Rollup's `input` option to an object format.
18+
*
19+
* Rollup accepts `input` as a string, array, or object, but Vite's `mergeConfig`
20+
* doesn't handle cross-type merging (e.g., merging an array with an object).
21+
* This function normalizes any input format to an object so entries can be merged.
22+
*
23+
* @see https://rollupjs.org/configuration-options/#input
24+
*/
25+
export const normalizeRollupInput = (input: Rollup.InputOption | undefined): Record<string, string> => {
26+
if (input == null) {
27+
return {}
28+
}
29+
30+
if (typeof input === 'string') {
31+
return { [getAliasName(input)]: input }
32+
}
33+
34+
if (Array.isArray(input)) {
35+
return Object.fromEntries(input.map((entry) => [getAliasName(entry), entry]))
36+
}
37+
38+
// Already an object
39+
return input
40+
}
41+
42+
/**
43+
* Merges new entries into an existing Rollup `input` option. Use this when you don't
44+
* control the existing `input` and need to add new entries without clobbering.
45+
*/
46+
export const mergeRollupInput = (
47+
existing: Rollup.InputOption | undefined,
48+
newEntries: Record<string, string>,
49+
): Record<string, string> => {
50+
return {
51+
...normalizeRollupInput(existing),
52+
...newEntries,
53+
}
54+
}

packages/vite-plugin-react-router/src/plugin.ts

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Plugin, ResolvedConfig } from 'vite'
66
import { glob } from 'tinyglobby'
77

88
import { version, name } from '../package.json'
9+
import { mergeRollupInput } from './lib/rollup'
910

1011
export interface NetlifyPluginOptions {
1112
/**
@@ -99,42 +100,49 @@ export function netlifyPlugin(options: NetlifyPluginOptions = {}): Plugin {
99100
let resolvedConfig: ResolvedConfig
100101
let isProductionSsrBuild = false
101102
let currentCommand: 'build' | 'serve' | undefined
103+
102104
return {
103105
name: 'vite-plugin-netlify-react-router',
104-
config(config, { command, isSsrBuild }) {
106+
config(_config, { command, isSsrBuild }) {
105107
currentCommand = command
106108
isProductionSsrBuild = isSsrBuild === true && command === 'build'
107109
if (isProductionSsrBuild) {
108-
// Replace the default SSR entrypoint with our own entrypoint (which is imported by our
109-
// Netlify function handler via a virtual module)
110-
config.build ??= {}
111-
config.build.rollupOptions ??= {}
112-
config.build.rollupOptions.input = {
110+
// Server bundle entry for our own server entry point (which is imported by our Netlify
111+
// function handler via a virtual module), while preserving any existing rollup input
112+
// entries (e.g., from react-router's prerender).
113+
const functionHandlerInput = {
113114
[FUNCTION_HANDLER_CHUNK]: FUNCTION_HANDLER_MODULE_ID,
114115
}
115-
config.build.rollupOptions.output ??= {}
116-
if (Array.isArray(config.build.rollupOptions.output)) {
117-
console.warn(
118-
'Expected Vite config `build.rollupOptions.output` to be an object, but it is an array - overwriting it, but this may cause issues with your custom configuration',
119-
)
120-
config.build.rollupOptions.output = {}
121-
}
122-
config.build.rollupOptions.output.entryFileNames = '[name].js'
123-
124-
// Configure for Edge Functions if enabled
125-
if (edge) {
126-
config.ssr = {
127-
...config.ssr,
128-
target: 'webworker',
129-
// Bundle everything except Node.js built-ins (which are supported but must use the `node:` prefix):
130-
// https://docs.netlify.com/build/edge-functions/api/#runtime-environment
131-
noExternal: /^(?!node:).*$/,
132-
resolve: {
133-
...config.resolve,
134-
conditions: ['worker', 'deno', 'browser'],
116+
// We use `mergeRollupInput` because Vite (even with `mergeConfig`) doesn't handle
117+
// cross-type merging for `rollupOptions.input` (string/array/object).
118+
const mergedInput = mergeRollupInput(_config.build?.rollupOptions?.input, functionHandlerInput)
119+
120+
const configChanges = {
121+
build: {
122+
rollupOptions: {
123+
input: mergedInput,
124+
output: {
125+
entryFileNames: '[name].js',
126+
},
135127
},
136-
}
128+
},
129+
// Additional config needed for Edge Functions if enabled
130+
...(edge
131+
? {
132+
ssr: {
133+
target: 'webworker' as const,
134+
// Bundle everything except Node.js built-ins (which are supported but must use the `node:` prefix):
135+
// https://docs.netlify.com/build/edge-functions/api/#runtime-environment
136+
noExternal: /^(?!node:).*$/,
137+
resolve: {
138+
conditions: ['worker', 'deno', 'browser'],
139+
},
140+
},
141+
}
142+
: {}),
137143
}
144+
145+
return configChanges
138146
}
139147
},
140148
async resolveId(source, importer, options) {

packages/vite-plugin-react-router/vitest.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { defineProject } from 'vitest/config'
66
export default defineProject({
77
plugins: [],
88
test: {
9-
include: ['./__tests__/*.{js,jsx,tsx,ts}'],
9+
include: ['./__tests__/*.{js,jsx,tsx,ts}', './src/**/*.test.{js,jsx,tsx,ts}'],
1010
globals: true,
1111
},
1212
})

removeMe.spec.ts

Lines changed: 0 additions & 5 deletions
This file was deleted.

tests/e2e/fixtures/react-router-edge-site/react-router.config.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,5 @@ export default {
44
// Config options...
55
// Server-side render by default, to enable SPA mode set this to `false`
66
ssr: true,
7-
// TODO(serhalp) Revisit this if RR team changes their minds:
8-
// https://github.com/remix-run/react-router/issues/13226#issuecomment-2776672461.
9-
// prerender: ['/prerendered'],
7+
prerender: ['/prerendered'],
108
} satisfies Config

0 commit comments

Comments
 (0)