fix(config): warn when App Router projects configure i18n by camc314 · Pull Request #3398 · cloudflare/vinext

camc314 · 2026-09-22T18:48:08Z

Warn when a project with app or src/app configures i18n in its Next.js config, matching Next.js's diagnostic that this configuration is unsupported in App Router. The warning uses the actual config filename and existing deduplication; Pages-only projects receive no warning.

Adds regressions for both App Router directory locations, config filenames, and repeated resolution, based on the Next.js i18n-hybrid suite.

The source commit records all 10 upstream i18n-hybrid tests and 240 next-config tests passing, along with targeted checks and package build. Tests were not rerun in this worktree because its local vite-plus dependency is not installed.

Match Next.js's diagnostic that next.config i18n applies to Pages Router and is unsupported by App Router. Detect both app and src/app, use the actual config filename, and reuse the existing warning deduplication. Pages-only projects retain their supported i18n configuration without a warning. Validation: - Original v16.2.6 i18n-hybrid suite: 10/10 passed. Routing already passed; the previously missing warning assertion now passes as well. - New root/src-app warning regressions fail before the change, verify no warning without App Router, and verify repeated resolution warns once. - Complete next-config suite: 240 passed. - Targeted vp check and vinext package build pass.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

pkg-pr-new · 2026-09-22T18:49:14Z

Open in StackBlitz

@vinext/cloudflare

npm i https://pkg.pr.new/@vinext/cloudflare@3398

create-vinext-app

npm i https://pkg.pr.new/create-vinext-app@3398

@vinext/types

npm i https://pkg.pr.new/@vinext/types@3398

vinext

npm i https://pkg.pr.new/vinext@3398

@cloudflare/workers-response-store

npm i https://pkg.pr.new/@cloudflare/workers-response-store@3398

commit: 5abbd3b

github-actions · 2026-09-22T18:53:13Z

Performance benchmarks

Compared 5abbd3b against base 087f8d6 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	142.3 KB	142.3 KB	⚫ -0.0%
Client entry size (gzip)	vinext	129.7 KB	129.7 KB	⚫ -0.0%
Dev server cold start	vinext	3.31 s	3.31 s	⚫ +0.0%
Production build time	vinext	3.57 s	3.54 s	⚫ -0.7%
RSC entry closure size (gzip)	vinext	127.9 KB	127.9 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	226.0 KB	226.1 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

james-elicx · 2026-09-22T21:55:37Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T21:57:25Z

    ? await resolveNextConfigInput(buildConfigMetadata.nextConfig, PHASE_PRODUCTION_BUILD)
    : await loadNextConfig(root, PHASE_PRODUCTION_BUILD);
-  const resolvedNextConfig = await resolveNextConfig(rawNextConfig, root);
+  const resolvedNextConfig = await resolveNextConfig(rawNextConfig, root, { hasAppDir: isApp });


[P2] Honor the configured active router in the CLI preflight

isApp comes from hasAppDir(root) before buildConfigMetadata.routeRootConfig is considered. Consequently, vinext build for a hybrid tree configured with vinext({ disableAppRouter: true }) still emits this App Router warning during the CLI resolution; the later plugin resolution with hasAppDir: false cannot retract it. The new test uses createBuilder() directly, so it does not exercise this CLI path. Please derive the flag from the route-root metadata (including disableAppRouter and a custom appDir) before passing it here.

ask-bonk · 2026-09-22T21:57:29Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:05:42Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:10:44Z

+  const isApp =
+    !routeRootConfig?.disableAppRouter &&
+    (routeRootConfig?.appDir
+      ? fs.existsSync(path.join(path.resolve(root, routeRootConfig.appDir), "app"))


[P2] Resolve appDir from the effective Vite root

VinextOptions.appDir is documented as relative to the Vite root, and the plugin resolves it against config.root in index.ts. This preflight instead resolves it against the CLI cwd. For example, with root: "frontend", vinext({ appDir: "." }), and frontend/app, the plugin activates App Router but this computes isApp = false, suppressing the warning. The same flag also controls React upgrading and hybrid-build setup below, so the CLI can select the wrong build path. Please carry the loaded config's effective root into this detection (ideally sharing the plugin's route-root resolution); the new custom-directory test only covers the cwd-as-root case.

ask-bonk · 2026-09-22T22:10:48Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:17:39Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:23:08Z

  // in its config() hook for each case, so cloudflare() and the plain Node SSR
  // build both work correctly.
-  const isHybrid = isApp && hasPagesDir();
+  const isHybrid = isApp && hasPagesDir(buildConfigMetadata.effectiveRoot);


[P2] Detect Pages from the configured route base

When appDir is set, it is the base containing both app/ and pages/ (and the plugin derives both directories from it), but this check still looks under effectiveRoot. In the new custom fixture, custom/app makes isApp true while the untouched root pages/ makes isHybrid true, so the subsequent vinext({ disableAppRouter: true }) build includes root Pages routes that the configured plugin intentionally ignores. Resolve the Pages directory from the same routeRootConfig/base directory as the App directory, and carry that base into the secondary build.

ask-bonk · 2026-09-22T22:23:12Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:28:44Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:30:45Z

          mode: buildMode,
          configFile: false,
-          plugins: [...userTransformPlugins, vinext({ disableAppRouter: true })],
+          plugins: [...userTransformPlugins, vinext({ appDir: routeBase, disableAppRouter: true })],


[P2] Run the hybrid Pages build from the effective Vite root

Passing the absolute routeBase fixes route discovery, but this vite.build still receives root = process.cwd() above. For the new root: "frontend" hybrid case, the primary build writes under frontend/dist and loads frontend/next.config.mjs, while this pass writes dist/server/entry.js at the CLI cwd and searches that cwd for next.config.*. The command therefore exits successfully with split artifacts and a Pages bundle missing the effective project's Next config. Use buildConfigMetadata.effectiveRoot as this build's root (and keep the post-build paths on that same root); the test should assert the resulting hybrid artifacts, not only the log line.

ask-bonk · 2026-09-22T22:30:49Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:37:08Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:39:03Z

    ? await resolveNextConfigInput(buildConfigMetadata.nextConfig, PHASE_PRODUCTION_BUILD)
    : await loadNextConfig(root, PHASE_PRODUCTION_BUILD);
-  const resolvedNextConfig = await resolveNextConfig(rawNextConfig, root);
+  const resolvedNextConfig = await resolveNextConfig(rawNextConfig, root, {


[P2] Read the warning config from the effective Vite root

hasActiveAppRouter is now correctly derived from buildConfigMetadata.effectiveRoot, but rawNextConfig and this filename lookup still use the CLI cwd. With root: "frontend", a root-level next.config.mjs containing i18n and an i18n-free frontend/next.config.mjs, the preflight emits an unsupported-App-Router warning even though the active project's config does not configure i18n. The inverse relies on the later plugin pass instead of this preflight. Preserve the existing build config resolution if needed, but load/resolve the config used for this diagnostic from effectiveRoot; the new Vite-root test avoids the disk-config case by supplying inline nextConfig.

ask-bonk · 2026-09-22T22:39:07Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:45:59Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:49:47Z

+  warnDeprecatedConfigOptions(
+    config,
+    root,
+    options.hasAppDir ?? ["app", "src/app"].some((dir) => fs.existsSync(path.join(root, dir))),


[P2] Propagate active router state to deploy config resolution

This fallback makes every caller that omits hasAppDir treat a conventional directory as active, but the Cloudflare deploy preflight at packages/cloudflare/src/deploy.ts:2048 calls resolveNextConfig(rawNextConfig, info.root) even though it has already loaded viteConfigMetadata.routeRootConfig. Consequently, vinext({ disableAppRouter: true }) with an app/ directory emits this unsupported-App-Router warning during vinext-cloudflare deploy, while a configured custom App Router can be missed (notably with --skip-build). Please derive the active App Router from the deploy metadata and pass it here, and audit the other non-plugin callers for the same distinction.

ask-bonk · 2026-09-22T22:49:50Z

Posted 1 actionable inline finding.

github run

james-elicx · 2026-09-22T22:56:34Z

/bigbonk review for issues

Time limit: 8 minutes. Complete the review and post all findings within that limit.

ask-bonk · 2026-09-22T22:59:13Z

    // The executed Vite config is authoritative. Source scans cannot see
    // imported or composed cache objects and must not suppress valid metadata.
    cacheConfig: await findVinextCacheConfigInPlugins(plugins),
+    effectiveRoot: path.resolve(root, loaded?.config.root ?? "."),


[P2] Resolve roots after Vite config hooks

loadConfigFromFile() only loads the raw exported config; it does not run plugin config hooks. If an earlier Vite plugin returns { root: "frontend" }, vinext itself receives frontend as its root, but this metadata pass keeps info.root, so hasActiveAppRouter checks the wrong tree and the i18n warning can be missed or emitted for an inactive app. The CLI metadata loader has the same pattern. Please obtain the resolved Vite root (after config hooks), or otherwise share the root that vinext actually receives, and add coverage for a plugin-supplied root.

ask-bonk · 2026-09-22T22:59:15Z

Posted 1 actionable inline finding.

github run

Copilot AI lite review requested due to automatic review settings September 22, 2026 18:48

Copilot AI reviewed Sep 22, 2026

fix(config): warn only for active App Router i18n

9e661d5

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): honor active App Router in CLI i18n preflight

813034c

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): resolve CLI router state from Vite root

6d956f7

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): share configured route base with hybrid build

65e9716

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): isolate i18n warning from build routing

f05781f

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): read i18n warning from effective Vite root

c8c92f5

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

fix(config): scope deploy i18n warning to active App Router

5abbd3b

ask-bonk Bot reviewed Sep 22, 2026

View reviewed changes

Conversation

camc314 commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

pkg-pr-new Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance benchmarks

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

james-elicx commented Sep 22, 2026

Uh oh!

ask-bonk Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Sep 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pkg-pr-new Bot commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading