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

Template literal validation #10492

Merged
merged 3 commits into from
Sep 26, 2019

Conversation

Basaingeal
Copy link
Contributor

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

I added a custom validation rule to the definition of TemplateLiteral in packages\babel-types\test\builders\es2015\templateElement.js to check that the number of quasis is exactly one more than the number of expressions.

I'm open to suggestions about the wording of the TypeError. I based it off of the number of quasis based off the wording of the issue, but I'm not sure if it needs to be in more layman's terms or more generalized or not.

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 24, 2019

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

Copy link
Member

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Thanks for starting this!

packages/babel-types/src/definitions/es2015.js Outdated Show resolved Hide resolved
).toThrowErrorMatchingSnapshot();

expect(() =>
t.templateLiteral({}, ["baz"]),
Copy link
Member

Choose a reason for hiding this comment

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

The first needs to be an array, and the second an array of expressions (not a string).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those tests are put there intentionally to show that the previous validators still trigger and that the only time the new validation rule triggers is when that's the only thing wrong.

I can remove it though if it's considered irrelevant.

Copy link

@hg-pyun hg-pyun left a comment

Choose a reason for hiding this comment

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

Looks good.

@nicolo-ribaudo
Copy link
Member

Thanks!

Co-authored-by: Michael J. Currie <michaeljcurrie136@gmail.com>
@nicolo-ribaudo nicolo-ribaudo merged commit 66062c2 into babel:master Sep 26, 2019
@nicolo-ribaudo
Copy link
Member

@Basaingeal I added Co-authored-by to the commit message to link the commit to your GitHub accout, but I suggest configuring it so that both your emails get linked!

@Basaingeal Basaingeal deleted the template-literal-validation branch September 26, 2019 18:02
@Basaingeal Basaingeal restored the template-literal-validation branch September 26, 2019 18:03
@Basaingeal Basaingeal deleted the template-literal-validation branch September 26, 2019 18:06
@tsareg
Copy link

tsareg commented Oct 21, 2019

Hey folks, after upgrading to recent @babel/types (namely 7.6.3) I started to get:
"Number of TemplateLiteral quasis should be exactly one more than the number of expressions.
Expected 2 quasis but got 1"

The code I'm trying is dynamic import with @loadable/component:

const AsyncIcon = loadable(props => import(`./icons/${props.name}.js`));

This was working fine with @babel/types 7.4.4.
Can you please explain what's wrong now with this code and what needs to be changed?

@nicolo-ribaudo
Copy link
Member

Could you open a new issue with your configuration?

@tsareg
Copy link

tsareg commented Oct 21, 2019

Could you open a new issue with your configuration?

I've created a bug for @loadable/component, see above. I was just wondering what have actually changed with the fix, I think this is where they use templateLiteral https://github.com/smooth-code/loadable-components/blob/master/packages/babel-plugin/src/properties/chunkName.js#L79-L88

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 24, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2020
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: types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@babel/types should validate the number of elements inside template literals
7 participants