Content-Length: 286960 | pFad | http://redirect.github.com/babel/babel/pull/16624

20 [Babel 8] Change `scope.{references,uids}` to `Set` by liuxingbaoyu · Pull Request #16624 · babel/babel · GitHub
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

[Babel 8] Change scope.{references,uids} to Set #16624

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Jul 9, 2024

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

In addition to changing scope.{references,uids} to Set, we also only create them at the top-level program scope, since we never use them in child scopes.
I'm not sure if these two properties are public, but it would be good to mention it in the migration guide.

@babel-bot
Copy link
Collaborator

babel-bot commented Jul 9, 2024

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

@liuxingbaoyu
Copy link
Member Author

In fact, there are some potential problems with the current behavior. For example, when calling childPath.scope.crawl(), invalid items in references will not be removed.

Comment on lines 395 to 402
references?: Set<string>;
globals: { [name: string]: t.Identifier | t.JSXIdentifier };
uids: { [name: string]: boolean };
uids?: Set<string>;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment saying that they are only defined in the program scope? Or maybe, we can simplify some methods by defining them in all scopes but using the same shared set.

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








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://redirect.github.com/babel/babel/pull/16624

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy