Skip to content

feat: Add upToScope parameter to hasBinding #17102

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

Merged
merged 4 commits into from
Mar 21, 2025

Conversation

liuxingbaoyu
Copy link
Member

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

@babel-bot
Copy link
Collaborator

babel-bot commented Jan 30, 2025

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

@nicolo-ribaudo nicolo-ribaudo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Jan 31, 2025
) {
if (!name) return false;
const upToScope = (opts as { upToScope?: Scope })?.upToScope;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Let's move opts normalization here so we don't have to do the typecast.

if (scope.hasOwnBinding(name)) {
return true;
}
} while ((scope = scope.parent));

if (!noUids && this.hasUid(name)) return true;
if (!noGlobals && Scope.globals.includes(name)) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you think noGlobals and noUids should interact with the upToScope option? The current behaviour is to completely ignore them when upToScope is configured, as if they were both true.

Copy link
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.

The updated behaviour looks good to me.

@nicolo-ribaudo nicolo-ribaudo added this to the v7.27.0 milestone Feb 7, 2025
@nicolo-ribaudo nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Feb 7, 2025
@nicolo-ribaudo nicolo-ribaudo merged commit c51cffd into babel:main Mar 21, 2025
55 checks passed
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 21, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: traverse (scope) PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scope.prototype.hasBinding: accept a "top" scope for search
4 participants