Conversation
publint suggests it wherever a package ships an ESM build, and it lets a bundler drop modules a consumer never references. Applied to the 23 packages that ship a dist; examples and module-compat are exempt, as neither declares an exports map nor is resolvable as a dependency. The flag is a promise — importing a module and not using it cannot change behaviour — so it was verified before being made. A TypeScript AST scan of every top-level statement in packages/*/src found one violation: SES built its provider Lambda's Runtime at module scope, and CDK's Runtime constructor appends to the shared Runtime.ALL. It is now built inside the function that needs it. Document the invariant in AGENTS.md alongside the sibling packaging rules. Nothing enforces it yet (#481).
CoverageOverall line coverage: 99.39% across 25 package(s).
|
|
Not sure if this is worth it. We know this project is never bundled since it is a dev dependency only. The lazy loaded change in SES is more complex than the eager loaded module singleton it replaced |
|
Closing unmerged. The change is correct but not worth its maintenance cost.
And upstream settles it anyway. Silencing a publint suggestion is not a reason on its own. That heuristic is aimed at libraries that ship to bundlers, which this one does not. Suppressing advice that does not apply is the same reflex that produced #465. The cost is not zero, which is what tips it: an invariant spanning 23 packages with nothing enforcing it, a rule in What survivesThe audit was still worth running — it found one real defect, now split out as #483: If this comes backIf |
1 participant
Closes the last outstanding publint suggestion. Follow-up: #481.
Why
publintsuggests"sideEffects"wherever a package ships an ESM build. The field tells a bundler that importing a module and not using it cannot change program behaviour, which licenses it to drop unreferenced modules — tree-shaking.Scope
Applied to the 23 packages that ship a
dist.examplesandmodule-compatare exempt: neither declares anexportsmap, and neither is resolvable as a dependency, so the field would be meaningless there.check:exportsnow reports zero suggestions across all 22 gated projects (verified with--skip-nx-cache).The flag is a promise, so it was verified before being made
Declaring
"sideEffects": falseon a package that does have module-load effects is worse than not declaring it — the bundler drops the module and its effect, and the bug appears only in bundled builds.Every top-level statement in
packages/*/src/**/*.tswas parsed with the TypeScript compiler API and classified.core,logs,custom-resourcesandcdk-testingare entirely clean. Of 74 flags elsewhere, all but one resolved to inert data once the callee was read:Duration.minutes/hours/days(...)static minutes(a){return new Duration(a, …)}— purenew ServicePrincipal(...)this.service/this.optsonlynew RegExp(...), template literals, defaults objectsstringConstraint({...})Symbol.for(...)brandsThe one real violation
packages/ses/src/activation.tsbuilt its provider Lambda's runtime at module scope:CDK's
Runtimeconstructor ends withRuntime.ALL.push(this)— so merely importing@composurecdk/sesmutated CDK's shared static. It is now built insideactivateReceiptRuleSet, the one function that needs it. The magic-string form is retained deliberately (usingRuntime.NODEJS_24_Xwould pull the package'saws-cdk-libfloor up to whatever release added the enum member); the comment moved with it.Per-call construction is what CDK itself does in
determineLatestNodeRuntime, and it is safe here:Runtime.ALLhas exactly one consumer inaws-cdk-lib,LayerVersion, which compares it by identity (props.compatibleRuntimes === Runtime.ALL ? undefined : …), so extra entries can never reach synthesised output.Detail worth knowing
Bundlers resolve
sideEffectsfrom the nearest enclosingpackage.json. Fordist/esm/index.jsthat isdist/esm/package.json— the dialect markertshywrites — not the package root. Confirmedtshypropagates the field into both markers, so this is effective rather than inert:Documentation
One bullet in
AGENTS.mdunder "Publishing & module format", alongside the sibling packaging rules (no import.meta,no instanceof, ADR-0018 prop typing). No ADR — this applies an existing decision rather than making a new one, and ADR-0007's enforcement table deliberately gains no row until a mechanism actually exists behind it.Honest sizing
aws-cdk-libdeclares nosideEffectsand is CommonJS, and it dominates any consumer's bundle — so this does not meaningfully change what a CDK app can tree-shake. It is correct, free metadata that silences a linter suggestion. That assessment is why #481 proposes a six-line assertion in the existingmodule-compatsuite rather than a custom ESLint rule: a syntactic rule would flag ~86 sites of which ~85 are inert, and would not have caught this one anyway.Verification
npm run verifypasses end to end.🤖 Generated with Claude Code