Skip to content

compat-eslint: lazy parent β€” defer parent resolution to first .parent read#96

Merged
johnsoncodehk merged 2 commits into
masterfrom
refactor/lazy-parent
May 3, 2026
Merged

compat-eslint: lazy parent β€” defer parent resolution to first .parent read#96
johnsoncodehk merged 2 commits into
masterfrom
refactor/lazy-parent

Conversation

@johnsoncodehk
Copy link
Copy Markdown
Owner

Summary

materialize(tsNode, ctx) previously walked up the TS parent chain and built every ancestor top-down so the leaf could be returned with a fully-resolved parent. Rules that never read .parent (e.g. JSX-attribute counters, type-checker leaf reads) paid for the entire chain anyway.

LazyNode.parent is now a lazy getter backed by a PARENT_UNSET sentinel; first read calls resolveParent, which walks the TS chain incrementally β€” one ancestor per .parent step. Bottom-up materialize constructs ONLY the requested leaf with parent: undefined. Top-down construction is unchanged β€” slot getters still pass this as the parent, so the constructor sets _parent directly and .parent reads stay O(1).

Three details that fell out of the design

  • Routers read tsNode.parent.kind directly. Routers that previously checked parent._ts.kind (BindingElement, Method/Get/Set accessor) now read the TS parent β€” the same node either way for these kinds, and routers must work without a resolved parent under the lazy-leaf path.
  • convertChild re-points a cached child's _parent when it's still PARENT_UNSET. Each TS node has exactly one ESTree parent, so the slot owner who's calling us IS that parent. Without this, ChainExpression's expression slot leaves the inner Identifier pointing at PARENT_UNSET and resolveParent drills to the wrong wrapper. Caught by prefer-rest-params regressing on arguments?.length.
  • Parent getter prefers a side-effect parent over the resolveParent return value. Wrappers like ChainExpression occupy the same TS slot as their inner expression, so resolveParent returns the outer wrapper β€” but the inner slot's convertChild already wrote the correct intermediate parent via the setter.

Module-level currentCtx lets routers call convertChild with parent: undefined and still resolve ctx for the cache lookup. Set on entry to materialize, restored on exit.

Numbers (Dify cold lint, 3 runs)

master lazy delta
Wall 13.55–13.88s 10.96–11.85s ~18% faster
User CPU ~21s ~17.5s ~17% faster
Node count 174,367 (60 kinds) 138,046 (48 kinds) ~21% fewer

JSXAttribute 24k→16.6k, JSXElement 15k→10.4k, JSXOpeningElement 8k→4.6k — those JSX ancestors are now built only when the rule actually reads .parent.

Net βˆ’206 lines in lazy-estree.ts.

Test plan

  • lazy-estree.test β€” incl. ~67k-assert dogfood parity sweep
  • predicate-coverage 152/152
  • bench 107 rules Γ— 33 files β€” clean parity
  • dogfood 107 rules Γ— 30 files β€” 0 divergences, 0 crashes
  • scope-compat 24/24, selector-analysis, ts-ast-scan, compat-pipeline, jsx-react-x PARITY βœ“
  • Dify cold lint β€” 3 runs, ~18% wall improvement, 21% fewer nodes

… read

`materialize(tsNode, ctx)` previously walked up the TS parent chain and built
every ancestor top-down so the leaf could be returned with a fully-resolved
parent. Rules that never read `.parent` (e.g. JSX-attribute counters, type-
checker leaf reads) paid for the entire chain anyway.

`LazyNode.parent` is now a lazy getter backed by a PARENT_UNSET sentinel;
first read calls `resolveParent`, which walks the TS chain incrementally one
ancestor per `.parent` step. Bottom-up `materialize` constructs ONLY the
requested leaf with `parent: undefined`. Top-down construction is unchanged
β€” slot getters still pass `this` as the parent, so the constructor sets
`_parent` directly and `.parent` reads stay O(1).

Three details that fall out of the design:

- Routers that previously checked `parent._ts.kind` (BindingElement, Method/
  Get/Set accessor) now read `tsNode.parent.kind` directly. The TS parent is
  the same node either way for these kinds, and routers must work without a
  resolved parent under the lazy-leaf path.
- `convertChild` re-points a cached child's `_parent` when it's still
  PARENT_UNSET. Each TS node has exactly one ESTree parent, so the slot
  owner who's calling us IS that parent β€” without this, ChainExpression's
  `expression` slot leaves the inner Identifier pointing at PARENT_UNSET
  and `resolveParent` then drills to the wrong wrapper.
- Parent getter prefers a side-effect parent over the `resolveParent`
  return value: wrappers like ChainExpression occupy the same TS slot as
  their inner expression, so resolveParent returns the outer wrapper, but
  the inner slot's `convertChild` already wrote the correct intermediate
  parent via the setter.

Module-level `currentCtx` lets routers call `convertChild` with `parent:
undefined` and still resolve ctx for the cache lookup. Set on entry to
`materialize`, restored on exit.

Numbers (Dify cold lint, 3 runs):
- Wall: 13.55–13.88s β†’ 10.96–11.85s (~18% faster)
- User CPU: ~21s β†’ ~17.5s (~17% faster)
- Node count: 174,367 β†’ 138,046 (~21% fewer, 60 kinds β†’ 48 kinds)

Tests: lazy-estree (incl. ~67k-assert dogfood parity sweep), predicate-
coverage 152/152, bench (107Γ—33 clean parity), dogfood (107Γ—30, 0
divergences), scope-compat 24/24, selector-analysis, ts-ast-scan, compat-
pipeline, jsx-react-x β€” all pass. Net βˆ’206 lines in lazy-estree.ts.
Encapsulate the cache-hit re-point. Replace the cast-based reach into
`_parent` from `convertChild` with a dedicated `_setParentIfUnset`
method on LazyNode. Same behaviour, but the PARENT_UNSET sentinel stays
an implementation detail.

Add three targeted tests for the subtle pieces of the lazy-parent path
that the parity sweep covers in aggregate but doesn't pin in isolation:

  - ChainExpression: `arguments?.length` β€” Identifier.parent must be
    MemberExpression, not the outer ChainExpression wrapper. This is
    the regression that prompted the cache-hit re-point fix in the
    first place.
  - BindingElement collapse: `let [a] = [1]` β€” Identifier `a`'s parent
    must be ArrayPattern, not itself (BindingElement collapses into the
    inner Identifier; resolveParent's `parent._ts !== walker` check
    keeps the walk going).
  - ParenthesizedExpression collapse: `(x);` β€” Identifier `x`'s parent
    must skip past the parens to ExpressionStatement.

A future "simplify" of the getter / cache-hit / collapse logic now
fails with a clear message instead of a vague type-mismatch row in the
67k-assert dogfood sweep.
@johnsoncodehk johnsoncodehk merged commit 4548ac8 into master May 3, 2026
1 check passed
@johnsoncodehk johnsoncodehk deleted the refactor/lazy-parent branch May 27, 2026 17:27
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