Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reference] feat: switched Node to a discriminated union #56274

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Oct 31, 2023

Posting as reference for now. Still has build failures. Not ready for review.

Naming is not at all well-thought-out. Names like NodeBase are just what came to mind first.

The most breaking portion of this PR's change is that Node no longer has a parent: Node, because SourceFile has parent: undefined. In retrospect this probably could be split into >= two steps:

  1. Creating a discriminated union, keeping the technically incorrect parent: Node inside SourceFile
  2. Switching parent: Node to parent: undefined inside SourceFile

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 31, 2023
@Jack-Works
Copy link
Contributor

If you're interested, we have discussed this in the engine262 repo. engine262/engine262#215 (comment)

I really hope this can land, but I think there might be performance pushback.

@jakebailey
Copy link
Member

Yes; the previous iteration caused check time to triple: #54148

@JoshuaKGoldberg
Copy link
Contributor Author

Closing to keep the review queue small.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants