Skip to content

always throw when add missing helpers#10208

Merged
existentialism merged 1 commit into
babel:masterfrom
tanhauhau:tanhauhau/ensure-helper
Jul 12, 2019
Merged

always throw when add missing helpers#10208
existentialism merged 1 commit into
babel:masterfrom
tanhauhau:tanhauhau/ensure-helper

Conversation

@tanhauhau

@tanhauhau tanhauhau commented Jul 12, 2019 •
edited
Loading

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

The problem with current addHelper is that this line will get execute first, before knowing whether the helper existed:

// 2) will short circuit if `this.declarations[name]` exists
const declar = this.declarations[name];
if (declar) return t.cloneNode(declar);

// 1) will set this.declarations[name] no matter helper exists
const uid = (this.declarations[name] = this.scope.generateUidIdentifier(
   name,
));

so i added a helpers.ensure(name) before (1).

Since loadHelper is cached, it's okay to call it multiple times.

@babel-bot

babel-bot commented Jul 12, 2019

Copy link
Copy Markdown
Collaborator

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

@existentialism existentialism added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: core labels Jul 12, 2019
@existentialism
existentialism merged commit f5ca058 into babel:master Jul 12, 2019
@nicolo-ribaudo

nicolo-ribaudo commented Jul 12, 2019

Copy link
Copy Markdown
Member

Related: #10200

@snyk-bot snyk-bot mentioned this pull request Aug 20, 2019
Closed
@snyk-bot snyk-bot mentioned this pull request Sep 5, 2019
Open
This was referenced Sep 17, 2019
Closed
Open
Open
This was referenced Oct 3, 2019
Open
Open
Open
@lock lock Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 11, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jridgewell jridgewell jridgewell approved these changes
@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: core PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing helpers only throw once

5 participants

@tanhauhau @babel-bot @nicolo-ribaudo @jridgewell @existentialism