Skip to content

Avoid checking Scope.globals multiple times#16619

Merged
nicolo-ribaudo merged 1 commit intobabel:mainfrom
liuxingbaoyu:perf-hasBinding
Jul 9, 2024
Merged

Avoid checking Scope.globals multiple times#16619
nicolo-ribaudo merged 1 commit intobabel:mainfrom
liuxingbaoyu:perf-hasBinding

Conversation

@liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Jul 7, 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

Previously hasBinding was called on each parent scope, which resulted in multiple checks of Scope.globals and Scope.contextVariables.
This improvement is small, and I didn't notice a noticeable improvement in real-case-preset-env-typescript.

@liuxingbaoyu liuxingbaoyu added PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories pkg: traverse (scope) labels Jul 7, 2024
@liuxingbaoyu liuxingbaoyu changed the title Avoid checking Scope.globals multiple times. Avoid checking Scope.globals multiple times Jul 7, 2024
@babel-bot
Copy link
Collaborator

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

} else if (typeof opts === "boolean") {
noGlobals = opts;
}
if (this.parentHasBinding(name, opts)) 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.

Do we have other parentHasBinding usage? If not maybe we can deprecate and remove this method in Babel 8, as it is trivial to implement.

Copy link
Member Author

Choose a reason for hiding this comment

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

It has only this one use.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I would not label this as "performance", since the performance different is insignificant

@liuxingbaoyu liuxingbaoyu added PR: Polish 💅 A type of pull request used for our changelog categories and removed PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories labels Jul 8, 2024
@nicolo-ribaudo nicolo-ribaudo merged commit e0368a8 into babel:main Jul 9, 2024
@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 Oct 9, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2024
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: Polish 💅 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants