Skip to content

Fix update expression for exported bigints#14341

Merged
nicolo-ribaudo merged 6 commits intobabel:mainfrom
magic-akari:fix/issue-14340
Mar 10, 2022
Merged

Fix update expression for exported bigints#14341
nicolo-ribaudo merged 6 commits intobabel:mainfrom
magic-akari:fix/issue-14340

Conversation

@magic-akari
Copy link
Copy Markdown
Contributor

@magic-akari magic-akari commented Mar 9, 2022

Q                       A
Fixed Issues? Fixes #14340
Patch: Bug Fix? 👍
Major: Breaking Change? behavior of @babel/helper-simple-access changed
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@nicolo-ribaudo nicolo-ribaudo added area: modules PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: BigInt labels Mar 9, 2022
}

const simpleAssignmentVisitor = {
UpdateExpression: {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If someone accidentally updates @babel/helper-simple-access without also updating @babel/helper-module-transforms (this is a valid thing to do, according to the semver dependency versions), I fear that this will break their current code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes.
What can I do? Should I bump the version of the package?

Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo Mar 9, 2022

Choose a reason for hiding this comment

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

No, because we keep all the versions synchronized (and it would require a major bump).

It's annoying, but a solution until Babel 8 is to pass a includeUpdateExpression: boolean = true option to simplifyAccess, and only skip the UpdateExpression visitor when that option is set to false.

Comment thread packages/babel-plugin-transform-modules-commonjs/src/index.ts
Copy link
Copy Markdown
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.

for the new tests

@nicolo-ribaudo nicolo-ribaudo changed the title Fix update expression for bigints Fix update expression for exported bigints Mar 10, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit a7b1181 into babel:main Mar 10, 2022
@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 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: update Expression transform into wrong result in cjs

3 participants