Skip to content

refactor(analyze): keep the shadow rule in one helper - #623

Open
tycrimm wants to merge 1 commit into
vercel:mainfrom
tycrimm:refactor/shared-shadow-rule
Open

tycrimm wants to merge 1 commit into
vercel:mainfrom
tycrimm:refactor/shared-shadow-rule

Conversation

@tycrimm

@tycrimm tycrimm commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Follow-up to @styfle's review on #613: the shadow rule lived in both getKnownBinding and createStaticEvalVars, so a change to one wouldn't reach the other.

getKnownBinding(knownBindings, name) moves to module level and is now the only place that reads shadowDepth === 0:

  • createStaticEvalVars calls it instead of repeating the rule.
  • hasKnownBindingValue becomes getKnownBinding(...) !== undefined. Known binding values are always objects, so this matches the old binding && binding.shadowDepth === 0.
  • The four inline knownBindings.require.shadowDepth === 0 checks call hasKnownBindingValue('require'). The unguarded one in require-wrapper detection only runs under !isESM || job.mixedModules, the same condition that defines require, so it reads the same.

The remaining shadowDepth uses are the initial values and the scope enter/exit counters.

No behavior change, so refactor: and no release.

Tests. No new tests. test/static-eval-vars.test.js already checks the rule against the old eager copy, and breaking the helper fails it. Full jest --coverage locally shows the same 8 environment-only failures as main (native builds and fixtures), with function coverage unchanged at 96.51%.

🤖 Generated with Claude Code

getKnownBinding moves to module level and is now the only place that
reads shadowDepth === 0. createStaticEvalVars, hasKnownBindingValue, and
the inline require checks call it instead of repeating the rule, so a
change to it reaches all of them. No behavior change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tycrimm
tycrimm requested review from a team, icyJoseph, ijjk and styfle as code owners November 5, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant