fix(build): forward pageExtensions to Vite resolve.extensions#1649
Conversation
When the user configured `pageExtensions` with values beyond Vite's defaults (e.g. `["platform.tsx", "tsx", "mdx"]`), extensionless imports of files with those extensions failed to resolve and the build crashed with `Custom deploy script failed: undefined (1)`. Mirror Next.js / Turbopack `resolveExtensions` semantics: prepend the configured `pageExtensions` (each prefixed with `.`) to Vite's default extension list, preserving the user's priority so e.g. `.web.tsx` resolves before `.tsx`. Ported from Next.js: test/e2e/app-dir/resolve-extensions/ Refs: #1502
|
/bigbonk review |
commit: |
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-scoped fix. The helper function is correct, the tests are thorough, and the integration point in index.ts is minimal.
A few notes from the review:
Config merge semantics are correct. I verified Vite's resolveEnvironmentResolveOptions uses mergeWithDefaults (not mergeConfig), which means the plugin's resolve.extensions array replaces Vite's defaults wholesale rather than being concatenated. This is why buildViteResolveExtensions must manually include Vite's defaults as its second parameter — that design is correct.
Minor consideration (non-blocking): If the user explicitly sets resolve.extensions in their vite.config.ts (or another plugin does), Vite's runConfigHook uses mergeConfig between plugin config hook returns, which concatenates arrays. So the final resolved list could contain duplicates — e.g., if the user wrote resolve: { extensions: ['.tsx'] }, the resolved array would have .tsx twice. This is harmless (Vite tries them in order, first match wins, and deduplication isn't needed for correctness), but worth noting for anyone reading the code later. Not worth complicating the implementation for.
normalizePageExtensions edge case works correctly. The regex /^\.+/ strips leading dots before the function re-adds a single . prefix in buildViteResolveExtensions. Input like ".platform.tsx" correctly normalizes to "platform.tsx" → ".platform.tsx". The test at line 58-62 covers this.
Tests are solid. Good coverage of the priority ordering, deduplication, multi-segment extensions, and the integration test that exercises the actual plugin config() hook with a tmpdir fixture.
|
Approved PR #1649. Here's a summary of the review: Verdict: Approved The fix is clean and correct.
|
1 participant
Summary
Fixes #1502.
When
next.config.{js,ts}configuredpageExtensionswith values beyond Vite's defaults (for example["platform.tsx", "tsx", "mdx"]), extensionless imports of files with those extensions failed to resolve and the build crashed withCustom deploy script failed: undefined (1).This change mirrors Next.js / Turbopack
resolveExtensionssemantics: the user-configuredpageExtensionsare prepended (each prefixed with.) to Vite's default extension list, preserving the user's priority — so.web.tsxresolves before.tsxand.platform.tsxresolves before.tsx, matching how Turbopack/webpack pick extension priority in Next.js.The page scanner already accepted
pageExtensions; this PR fills in the remaining gap on the Vite resolver side.Ported from Next.js:
test/e2e/app-dir/resolve-extensions/.Test plan
vp test run tests/page-extensions-resolve.test.ts(new tests cover the helper and verify the Vite config wiring)vp test run tests/file-matcher.test.ts tests/page-extensions-routing.test.ts tests/build-optimization.test.ts(no regressions)vp test run tests/next-config.test.ts tests/next-config-extensions.test.ts(config tests still pass)vp check tests/page-extensions-resolve.test.ts packages/vinext/src/routing/file-matcher.ts packages/vinext/src/index.ts