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

fix: handle tokens for invalid template element #14055

Merged
merged 1 commit into from Dec 14, 2021
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Dec 14, 2021

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

This PR fixes a regression introduced at #13919. We skip evaluating the template value when a template element is invalid, i.e. it may contain invalid Unicode escape or octal escape.

Since Babel parser updates have broken prettier for several times (https://github.com/babel/babel/issues?q=is%3Aissue+label%3A%22i%3A+regression%22+prettier+label%3A%22pkg%3A+parser%22), I plan to add prettier-e2e test in another PR so hopefully a babel parser patch release will not break prettier in the future.

@JLHwung JLHwung added PR: Bug Fix 🐛 pkg: parser i: regression labels Dec 14, 2021
@babel-bot
Copy link
Collaborator

@babel-bot babel-bot commented Dec 14, 2021

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

@@ -135,7 +135,7 @@ function babel7CompatTokens(tokens) {
// ends with '`'
templateElementEnd = end - 1;
templateElementEndLoc = createPositionWithColumnOffset(loc.end, -1);
templateValue = value.slice(1, -1);
templateValue = value === null ? null : value.slice(1, -1);
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo Dec 14, 2021

Choose a reason for hiding this comment

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

Nit: we can use value?.slice

Copy link
Contributor Author

@JLHwung JLHwung Dec 14, 2021

Choose a reason for hiding this comment

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

I don't think so. We need returning null when value is null.

@JLHwung JLHwung merged commit 7794201 into babel:main Dec 14, 2021
26 of 27 checks passed
@JLHwung JLHwung deleted the fix-14054 branch Dec 14, 2021
@github-actions github-actions bot added the outdated label Mar 16, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: regression outdated pkg: parser PR: Bug Fix 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants