fix(config): drop three dead next.config entries and correct two stale comments - #6076
Conversation
…e comments
`optimizePackageImports` is not a no-op under Turbopack, despite what the
Next docs say. In 16.2.11 the list feeds `side_effect_free_packages` and is
force-appended to `transpiledPackages`, which also removes each entry from
the server externals set — and it overrides each listed package's own
`sideEffects` declaration rather than hinting. So a stale entry is not free.
Two entries are provably dead: `lodash` has zero import sites repo-wide and
is not a dependency of any package here (Next also already applies a
`lodash -> lodash/{{member}}` modularizeImports rule unconditionally), and
`@radix-ui/react-accordion`'s only occurrence in the repo was this line.
`prettier` in `transpilePackages` is dead for a different reason: nothing
imports it, it is in no package.json, and the repo formats with Biome. It
sits in node_modules only as a transitive dep. `transpilePackages` matches
on modules already in the graph, so this entry was transpiling nothing.
Also corrects two comments that claimed more than the code does:
- the `.map` header rule said it blocks access to sourcemaps; it only sets
`noindex` and a TTL. The maps ship publicly in the production image.
- the minimal-registry comment's numbers were badly stale (~247 tools ->
4,351; ~2,074 modules -> ~5,907) and its framing wrong: blocks are not a
co-equal cost, and the tool registry is reached through four redundant
client edges, not just providers/utils — which is why cutting only that
one edge buys a single module.
No behavior change intended: the removed entries were inert. Kept `zod`,
`reactflow`, `framer-motion`, and `streamdown`, which are real barrels with
real import sites — removing those changes tree-shaking hints and needs a
bundle-size measurement, not a guess.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Removes Adds a comment that Comment fixes: the dev minimal-registry blurb is updated with current module-count measurements and the four client edges into Reviewed by Cursor Bugbot for commit 9e7c380. Configure here. |
Greptile SummaryHygiene-only cleanup in
Confidence Score: 5/5Safe to merge; config hygiene with no remaining behavioral risk from the removed entries. Removed packages have no import sites or direct dependencies in apps/sim; transpilePackages remains populated; remaining edits are comment accuracy only.
|
| Filename | Overview |
|---|---|
| apps/sim/next.config.ts | Dead optimizePackageImports/transpilePackages entries removed; comments corrected; no runtime code paths changed. |
Reviews (1): Last reviewed commit: "fix(config): drop three dead next.config..." | Re-trigger Greptile
`vitest.setup.ts` loaded `@testing-library/jest-dom/vitest` unconditionally, so all 1,219 test files paid for it. jest-dom only registers DOM matchers (`toBeInTheDocument`, `toBeVisible`, ...), which are useless without a document — and 985 of those files declare `@vitest-environment node`. Guarding the import on `typeof document !== 'undefined'` loads it for exactly the files that can use it. All 7 files that actually call a jest-dom matcher are explicitly `@vitest-environment jsdom`, so none loses anything; verified by running them. Measured locally, same machine, full suite: baseline Duration 179.47s (setup 367.13s aggregate) guarded Duration 165.78s (setup 252.72s aggregate) 7.6% off the suite, 31% off aggregate setup time. On a node-only subset (50 files, n=3 each) it is 21%: 2.75s -> 2.16s median. Identical results either way: 1225 files / 16184 tests pass, plus one failure that reproduces on unmodified staging (cloud-review-tools.test.ts cannot find `rg` from its spawned python3 locally; CI installs ripgrep explicitly and it passes there). Also fixes two pieces of CI documentation that claimed more than the code does — the same class of thing #6076 cleaned up: - The step was named "Run tests with coverage" but runs `bun run test` = `vitest run`, with no `--coverage` anywhere. - The Codecov upload reads `apps/sim/coverage`, which nothing generates (vitest.config.ts declares no coverage provider). It uploads nothing and still reports success in ~1s because `fail_ci_if_error: false`. Annotated rather than deleted, since whether to enable coverage or drop the step is a call for the team, not a side effect of a perf change.
`vitest.setup.ts` loaded `@testing-library/jest-dom/vitest` unconditionally, so all 1,219 test files paid for it. jest-dom only registers DOM matchers (`toBeInTheDocument`, `toBeVisible`, ...), which are useless without a document — and 985 of those files declare `@vitest-environment node`. Guarding the import on `typeof document !== 'undefined'` loads it for exactly the files that can use it. All 7 files that actually call a jest-dom matcher are explicitly `@vitest-environment jsdom`, so none loses anything; verified by running them. Measured locally, same machine, full suite: baseline Duration 179.47s (setup 367.13s aggregate) guarded Duration 165.78s (setup 252.72s aggregate) 7.6% off the suite, 31% off aggregate setup time. On a node-only subset (50 files, n=3 each) it is 21%: 2.75s -> 2.16s median. Identical results either way: 1225 files / 16184 tests pass, plus one failure that reproduces on unmodified staging (cloud-review-tools.test.ts cannot find `rg` from its spawned python3 locally; CI installs ripgrep explicitly and it passes there). Also fixes two pieces of CI documentation that claimed more than the code does — the same class of thing #6076 cleaned up: - The step was named "Run tests with coverage" but runs `bun run test` = `vitest run`, with no `--coverage` anywhere. - The Codecov upload reads `apps/sim/coverage`, which nothing generates (vitest.config.ts declares no coverage provider). It uploads nothing and still reports success in ~1s because `fail_ci_if_error: false`. Annotated rather than deleted, since whether to enable coverage or drop the step is a call for the team, not a side effect of a perf change.
Summary
Three entries in
apps/sim/next.config.tswere inert. Removing them is behavior-neutral. Also corrects two comments that claimed more than the code does.This came out of an audit of build-speed levers. None of these is a performance fix — they are hygiene, and I'd rather say so than imply a win that isn't there.
The
optimizePackageImportsfindingThe Next 16.2.11 docs state, verbatim: "Turbopack automatically analyzes imports and optimizes them. It does not require this configuration." That sentence is not backed by the code. At tag
v16.2.11:crates/next-core/src/next_client/context.rs:362andnext_server/context.rs:586— the list feedsside_effect_free_packagesnext_server/context.rs:167— the list is force-appended totranspiled_packages, and:191then removes those entries from the server externals setturbopack-ecmascript/src/chunk/placeable.rs:235— the glob short-circuits ahead of each package's ownpackage.jsonsideEffectsfield. It is an override, not a hint.Next's own
turbopack-warning.jsalso does not list the option as Turbopack-unsupported. So a stale entry is not free: it costs transform work and silently overrides a package'ssideEffectsdeclaration.What was removed
lodash(optimizePackageImports)lodash → lodash/{{member}}modularizeImportsrule unconditionally.@radix-ui/react-accordion(same)node_modulesonly via transitive hoisting.prettier(transpilePackages)package.json, and the repo formats with Biome.transpilePackagesmatches modules already in the graph, so this transpiled nothing.What was deliberately left alone
zod,reactflow,framer-motion,streamdown— real barrels with real import sites (zodhas 389). Removing them removes genuineside_effect_free_packagestree-shaking hints and could grow the client bundle. That's a measurable tradeoff, not a cleanup.@radix-ui/react-*entries — each is a 2-file pre-bundled package, outside the option's documented purpose ("hundreds or thousands of modules"), so they're likely noise plussideEffects-override risk. But they are imported, so removal is a bundle-size question. Deferred on purpose.@react-email/*and@t3-oss/*intranspilePackages— these are where real transform work happens, and all four ship prebuilt JS meeting no documented criterion. Not touched: they were added empirically to fix real bugs, and the@t3-ossfailure mode is silently-undefinedclient env, which ships unnoticed.transpilePackagesarray must stay non-empty.webpack-config.jsgatesshouldIncludeExternalDirson!!config.transpilePackages, so emptying it would breakdev:webpackcompilingpackages/*/src. Individual removals are fine.Comment corrections
.mapheader rule was labelled "Block access to sourcemap files (defense in depth)". It setsx-robots-tag: noindexand a TTL — it blocks nothing, and the maps ship publicly in the production image (docker/app.Dockerfile:107).BLOCK_REGISTRYis 313. Blocks are not a co-equal cost —blocks/registry-mapsalone is ~349 modules and mostly rides in behind the tool registry. And the registry is reached via four redundant client-reachable edges, not justproviders/utils → tools/params, which is why cutting that one edge buys a single module.Type of Change
Testing
tsc --noEmitclean (exit 0),biome checkclean. Config-only change with no runtime code touched; the removed entries were inert by the mechanisms cited above. The CIBuild Appcheck exercises the real build.