[hyoban]

https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/constants.ts

[southpolesteve]

/bigbonk review this PR

[ask-bonk]

Review: Align next/constants shim with Next.js source

Good change — the shim now has 1:1 API parity with Next.js constants.ts. All values are correct and no exports are missing or extraneous. Type-checks clean.

A couple of things to consider:

1. Type declarations are now stale

packages/vinext/src/shims/next-shims.d.ts (lines 201-208) only declares the original 6 phase constants. With 200+ new exports, any consumer relying on the .d.ts (rather than directly importing the .ts file) won't see types for PAGES_MANIFEST, BUILD_MANIFEST, AdapterOutputType, etc. This should be updated in the same PR to stay in sync.

2. Tests only cover the original 6 phase constants

tests/shims.test.ts (line 5063-5072) still only asserts the 6 original phase constants. It would be good to add at least a spot-check for a few of the newly added exports (e.g. PAGES_MANIFEST, BUILD_MANIFEST, STATIC_PROPS_ID, CONFIG_FILES) to verify the shim is importable and values are correct.

3. process.features in Workers runtime

The CONFIG_FILES array (line 117-124) uses process?.features which is safe via optional chaining, but process.features.typescript is a Node.js-only API that won't exist on Cloudflare Workers. This means "next.config.mts" will never appear in the array at runtime on Workers. Since this constant isn't consumed by vinext internals (the config loader has its own file list), the practical impact is zero — but it's worth noting in case a third-party library depends on the full list. This is inherited from the Next.js source, so it's fine to keep as-is.

[ask-bonk]

Review posted successfully. Here's a summary of my findings:

The PR correctly aligns the next/constants shim with the upstream Next.js source — all values match, nothing is missing or extraneous, and it type-checks clean.

I flagged two follow-up items for the author:

1. Stale type declarations — next-shims.d.ts only declares the original 6 phase constants and needs updating to cover the ~60 new exports.

2. Tests need expanding — the existing test only checks the 6 phase constants; a few spot-checks on the newly added exports would be good.

I also left an inline note on the CONFIG_FILES constant about process.features not existing on Cloudflare Workers (inherited from Next.js source, no action needed).

github run

[hyoban]

Changed to align with next exports.

BTW, I'm not quite clear why the type needs to be redeclared.

[pkg-pr-new]

Open in StackBlitz

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

commit: 2818680

[james-elicx]

/bigbonk review this pr please.

[ask-bonk]

Review: Align next/constants shim with Next.js

This looks good. The shim now has complete API parity with Next.js 16's next/constants, the .d.ts declarations are in sync with the implementation, and the test strategy of comparing against the actual next package is solid.

All CI checks pass.

One minor concern below, otherwise ready to merge.

[ask-bonk]

Adding next as a root devDependency just for test assertions is a meaningful dependency to take on — it pulls in a large transitive tree (React, webpack internals, etc.) even though it's only used in one test.

That said, the payoff is real: comparing against the actual package guarantees the shim stays in sync without manually maintaining a list of expected exports. The tradeoff seems worth it.

Just noting that this means CI install times and lockfile churn will now track Next.js releases. If that becomes noisy, you could pin to an exact version instead of ^16.1.6.

[ask-bonk]

This is a clever approach — comparing the full module export surface against the real next package catches any drift automatically. The Symbol normalization via .description is the right call.

One thing to be aware of: since the shim's SYSTEM_ENTRYPOINTS is a new Set(...) and Next.js's is also a new Set(...), toEqual does deep equality on Sets, which should work correctly in Vitest. Just flagging in case a future Vitest version changes Set comparison semantics.

[ask-bonk]

Review submitted. Here's a summary of my findings:

Verdict: Approved

The PR correctly aligns the next/constants shim with the upstream Next.js 16 source. All ~60 exports match the published next@16.1.6 package, the .d.ts type declarations have been fully updated (addressing the earlier review feedback), and the test has been rewritten to compare the entire export surface against the real next package — which is a much more robust approach than manually checking individual values.

Key observations:

1. Constants implementation — 1:1 parity with the Next.js source. All values, types, and the enum are correct.

2. Type declarations updated — The .d.ts now covers all new exports, addressing the stale declarations issue from the prior review.

3. Test approach — Smart comparison against the actual next package with proper normalization for Symbol and module metadata (__esModule, default). This guarantees the shim stays in sync automatically.

4. New next devDependency — Adds next@^16.1.6 at the monorepo root solely for test assertions. This is a heavyweight dependency but the automated drift detection it enables is worth it. I left a note suggesting an exact pin if lockfile churn becomes noisy.

github run