Skip to content

fix: Improve type reasoning for visitors containing |#17899

Merged
JLHwung merged 2 commits intobabel:mainfrom
liuxingbaoyu:fix-get-union
Apr 16, 2026
Merged

fix: Improve type reasoning for visitors containing |#17899
JLHwung merged 2 commits intobabel:mainfrom
liuxingbaoyu:fix-get-union

Conversation

@liuxingbaoyu
Copy link
Copy Markdown
Member

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This resolves the issue where every nodeA|nodeB would be complained about by TS when strictFunctionTypes: true.
It also provides better type reasoning.
Due to the limitations of TypeScript, this can only be implemented within function calls.

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Mar 24, 2026

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61267

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Mar 24, 2026

Open in StackBlitz

commit: 289a88f

Comment on lines -25 to -34
export type TraverseOptions<S = t.Node> = {
scope?: Scope;
noScope?: boolean;
denylist?: string[];
shouldSkip?: (node: NodePath) => boolean;
} & Visitor<S>;

export type ExplodedTraverseOptions<S = t.Node> = TraverseOptions<S> &
ExplodedVisitor<S>;

Copy link
Copy Markdown
Member Author

@liuxingbaoyu liuxingbaoyu Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these generic types are incorrect.
ExplodedTraverseOptions also contains incorrect usage, because previously TraverseOptions included Visitor, while context#opts should only contain ExplodedVisitor.
In this PR, I adjusted TraverseOptions to not contain Visitor and removed ExplodedTraverseOptions.

Uh, I even realized that Visitor also includes ExplodedVisitor.

Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@liuxingbaoyu liuxingbaoyu changed the title fix: Supports visitors containing | fix: Better type reasoning for visitors containing | Mar 27, 2026
@liuxingbaoyu liuxingbaoyu changed the title fix: Better type reasoning for visitors containing | fix: Improve type reasoning for visitors containing | Mar 27, 2026
@liuxingbaoyu liuxingbaoyu changed the title fix: Improve type reasoning for visitors containing | fix: Improve type reasoning for visitors containing | Mar 27, 2026
@JLHwung JLHwung merged commit 39b57b4 into babel:main Apr 16, 2026
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants