fix(config): match deprecation warning parity

[james-elicx]

Summary

• match Next.js v16.2.6 warnings for explicitly configured deprecated options
• emit each warning once per process, matching Next.js Log.warnOnce
• filter only package-manager install noise from deploy-suite output while preserving application lifecycle diagnostics

Next.js parity

Addresses the two failures in test/e2e/deprecation-warnings/deprecation-warnings.test.ts from the latest deploy-suite report.

Validation

• focused config and deploy-script tests
• lifecycle log-filter shell probes
• scoped vp check
• vp run vinext#build
• independent review: no actionable findings

Full suites are deferred to CI.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 9779fe4

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[ask-bonk]

/bigbonk review — PR #2252 (deprecation warning parity)

Verified the implementation against Next.js v16 source (packages/next/src/server/config.ts → checkDeprecations / warnOptionHasBeenDeprecated) and the target test test/e2e/deprecation-warnings/deprecation-warnings.test.ts. The five warnings, their exact message strings, the property-path traversal (!== undefined semantics including falsy values), and the no-config-file silence all match Next.js precisely. vp check and the focused tests pass. No blocking issues found — the notes below are non-blocking.

Non-blocking observations

1. warnConfigOnce dedup is process-global and never reset in tests (next-config.ts:829).
emittedConfigWarnings is a module-level Set with no test reset hook. The three new tests only stay independent because they happen to use different config filenames (next.config.js vs next.config.mjs), so the dedup keys differ. This is correct parity with Next.js Log.warnOnce (process-global), but the test isolation is incidental. If a future test resolves these deprecated options at the default cwd, run-order could suppress a warning and flake the "matches Next.js warnings" assertion. Consider exporting a reset helper for tests, or clearing the set in afterEach.

2. "matches Next.js warnings" test depends on repo cwd having no next.config.* (next-config.test.ts:64).
It calls resolveNextConfig(config) with the default root = process.cwd() and asserts the next.config.js filename. This passes today only because the vinext repo root has no next.config.* (verified). It would break if such a file were ever added at the root. Passing an explicit temp root (as the "warns once" test does) would make it robust.

3. Partial parity — only 5 of Next.js's deprecation warnings are implemented.
Next.js checkDeprecations also warns for experimental.after, experimental.rootParams, eslint, images.domains, i18n (App Router), and experimental.browserDebugInfoInTerminal. This PR scopes to the five the target test covers, which is fine, but the PR title ("match deprecation warning parity") is broader than the actual coverage. Worth a code comment or follow-up note that the list is intentionally partial.

4. filter-e2e-install-log.sh lifecycle state machine relies on pnpm output ordering (filter-e2e-install-log.sh:5-13).
in_lifecycle_script flips on at the first > pkg ... postinstall header and only flips off at Done in ... using pnpm vN. Two assumptions:

• If install fails before emitting the Done in ... line (the deploy step tolerates a non-zero install via || true), in_lifecycle_script stays 1 for the remainder and no deprecated subdependencies/DEP0169 filtering happens. This fails open (keeps content), so it's safe but means the noise this PR targets could still leak in a failed-install run.
• The design assumes pnpm prints its own deprecated subdependencies found: summary after the final Done in line, while application lifecycle output precedes it. That matches current pnpm behavior but is a heuristic that could break on a pnpm output-format change. Acceptable for an internal deploy-harness filter; just flagging the coupling.

5. Filter step has no || true under set -euo pipefail (e2e-deploy.sh:627).
Unlike the install (run_pnpm ... || true), the filter pipeline can now abort the whole deploy if awk/the redirect fails. Awk failure is unlikely and aborting on a genuine write error is arguably correct, but it's a slightly stricter failure surface than the previous direct >> append.

Verdict

Implementation faithfully reproduces the Next.js v16 behavior the target e2e test checks. The notes above are hardening/maintainability suggestions, not correctness blockers.

[ask-bonk]

Review posted to PR #2252.

Summary

I reviewed PR #2252 (deprecation warning parity), verifying the implementation against the Next.js v16 source (packages/next/src/server/config.ts) and the target test test/e2e/deprecation-warnings/deprecation-warnings.test.ts.

Verdict: No blocking issues. The five warnings, exact message strings, !== undefined property-path semantics (including falsy values like instrumentationHook: false), and the no-config-file silence all match Next.js precisely. vp check and the focused tests pass.

Non-blocking findings posted:

1. emittedConfigWarnings Set is process-global with no test reset — test isolation is incidental (relies on differing config filenames). Correct parity with Log.warnOnce, but fragile.
2. "matches Next.js warnings" test depends on the repo cwd having no next.config.* (true today, but brittle vs. an explicit temp root).
3. Partial parity — only 5 of Next.js's deprecation warnings implemented (Next.js also warns for experimental.after, rootParams, eslint, images.domains, i18n, browserDebugInfoInTerminal). PR title is broader than coverage.
4. filter-e2e-install-log.sh awk state machine relies on pnpm output ordering and fails open if install errors before the Done in line.
5. Filter step lacks || true under set -euo pipefail — slightly stricter failure surface than the prior direct append.

All findings are hardening/maintainability suggestions, not correctness blockers — the implementation faithfully reproduces the Next.js behavior the target e2e test checks.

github run

[github-actions]

Performance benchmarks

Compared 9779fe4 against base cf9d601 using alternating same-runner rounds. Next.js was unchanged and skipped.

4 improved · 0 regressed · 2 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.8 KB	117.1 KB	🟢 -11.8%
Client entry size (gzip)	vinext	118.7 KB	112.5 KB	🟢 -5.2%
Dev server cold start	vinext	2.44 s	2.43 s	⚫ -0.6%
Production build time	vinext	3.07 s	2.92 s	🟢 -4.8%
RSC entry closure size (gzip)	vinext	92.2 KB	93.0 KB	⚫ +0.9%
Server bundle size (gzip)	vinext	166.9 KB	156.9 KB	🟢 -6.0%

View detailed results and traces

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

[james-elicx]

/bigbonk review

[ask-bonk]

/bigbonk review — PR #2252 (deprecation warning parity)

Independently verified the implementation against Next.js v16 source (packages/next/src/server/config.ts → warnOptionHasBeenDeprecated / checkDeprecations) and the target e2e test (test/e2e/deprecation-warnings/deprecation-warnings.test.ts). The five warnings, their exact message strings, the property-path traversal semantics, and the no-config silence all match Next.js. vp check is clean on the changed source and the focused suites pass (tests/next-config.test.ts, tests/check.test.ts, tests/e2e-deploy-script.test.ts — 347 tests).

Verdict: no blocking issues. This concurs with the earlier review on this PR. A few additional confirmations and one nuance below; all non-blocking.

Parity confirmations

• hasConfigProperty is safer than Next.js, not divergent. Next.js traverses with current[key] !== undefined and would throw on a null intermediate (e.g. experimental: null → null["instrumentationHook"]). vinext's isUnknownRecord(current) guard returns false instead. Both reach "not found" for primitive/missing intermediates, and a config that crashes Next.js is not a behavior we need to reproduce. Leaf null/false values warn in both (matching the e2e fixture's instrumentationHook: false expectation). Good.
• Message strings match Next.js v16 exactly, including the in ${configFileName}. suffix and the distinct instrumentationHook phrasing. The e2e test only uses toContain, so the absence of Next.js's ⚠ Log.warn prefix on the bare console.warn output is immaterial to the target assertions.
• Centralized in resolveNextConfig, so dev/prod/Workers paths all inherit the same behavior — no parity gap across the four request-handling entries.

Non-blocking notes (concurring with the prior review)

1. emittedConfigWarnings is process-global with no test reset. The two new next-config.test.ts cases stay independent only because their messages differ by configFileName (next.config.js vs next.config.mjs), so the dedup keys differ. This is correct parity with Log.warnOnce, but a future test that resolves the same options at the default cwd could be order-suppressed and flake. An exported reset helper (or afterEach clear) would make isolation explicit.
2. The "matches Next.js warnings" test depends on the repo root having no next.config.*. It asserts next.config.js as the filename via the default root = process.cwd(). True today; would break if a root config were ever added. Passing an explicit temp root (as the "warns once" test does) would harden it.
3. Partial parity by design. Only 5 of Next.js's checkDeprecations warnings are implemented (omits experimental.after, rootParams, eslint, images.domains, i18n, browserDebugInfoInTerminal). This covers exactly what the target e2e test checks, which is fine, but the PR title is broader than coverage — a code comment noting the list is intentionally partial would help future readers.
4. filter-e2e-install-log.sh fails open on a failed install. If pnpm install errors before emitting Done in … using pnpm vN, in_lifecycle_script stays 1 and no deprecated subdependencies/DEP0169 filtering happens for the rest of the stream. Safe (keeps content) but means the targeted noise could still leak in a failed-install run. The Done in … using pnpm v[0-9] anchor is also a heuristic coupled to current pnpm output formatting. Acceptable for an internal deploy-harness filter.
5. The filter pipeline has no || true under set -euo pipefail (e2e-deploy.sh:627), unlike the tolerated install (|| true). An awk/write failure would now abort the whole deploy. Awk failure is unlikely and aborting on a genuine write error is arguably correct, but it's a slightly stricter failure surface than the previous direct >> append.

Verdict

Faithfully reproduces the Next.js v16 behavior the target e2e test exercises. The notes above are hardening/maintainability suggestions, not correctness blockers.

[ask-bonk]

Review posted to PR #2252.

Summary

I independently reviewed PR #2252 (deprecation warning parity), verifying the implementation against the Next.js v16 source (packages/next/src/server/config.ts) and the target e2e test.

Verdict: No blocking issues.

What I verified directly:

• Message strings match Next.js v16 exactly (including the in ${configFileName}. suffix and the distinct instrumentationHook phrasing).
• hasConfigProperty is actually safer than Next.js — it guards intermediate null values that would crash Next.js's current[key] !== undefined traversal, while reaching identical results for the cases that matter (leaf false/null values warn in both, matching the e2e fixture's instrumentationHook: false).
• Logic is centralized in resolveNextConfig, so all four request-handling entries inherit it — no dev/prod parity gap.
• vp check clean on the changed source; focused suites pass (347 tests across the three test files).

Non-blocking findings (concurring with the earlier review on this PR):

1. emittedConfigWarnings is process-global with no test reset — isolation is incidental (differing config filenames).
2. The "matches Next.js warnings" test relies on the repo root having no next.config.*.
3. Partial parity by design (5 of Next.js's warnings); PR title broader than coverage.
4. filter-e2e-install-log.sh fails open on a failed install and couples to current pnpm output formatting.
5. The filter pipeline lacks || true under set -euo pipefail (slightly stricter failure surface than the prior append).

All are hardening/maintainability suggestions, not correctness blockers — the implementation faithfully reproduces the Next.js v16 behavior the target e2e test exercises.

github run