Skip to content

feat: Automatically generate cooked for templateElement#14757

Merged
liuxingbaoyu merged 7 commits into
babel:mainfrom
liuxingbaoyu:templateElement-cooked
Jul 21, 2022
Merged

feat: Automatically generate cooked for templateElement#14757
liuxingbaoyu merged 7 commits into
babel:mainfrom
liuxingbaoyu:templateElement-cooked

Conversation

@liuxingbaoyu

@liuxingbaoyu liuxingbaoyu commented Jul 14, 2022

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

I'm not sure if I need to check the cooked parameter with a value, please give your opinion.

Also I don't think we need to do the same in @babel/plugin-transform-template-literals, just update types.

@liuxingbaoyu liuxingbaoyu added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: types labels Jul 14, 2022
@babel-bot

babel-bot commented Jul 14, 2022

Copy link
Copy Markdown
Collaborator

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

Comment thread packages/babel-types/src/definitions/utils.ts

@JLHwung JLHwung left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread packages/babel-types/src/definitions/utils.ts
Comment thread packages/babel-types/test/builders/es2015/__snapshots__/templateElement.js.snap Outdated
Comment thread packages/babel-types/src/definitions/core.ts Outdated

let cooked = null;
try {
cooked = unraw(raw);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Q: should we throw if user provided inconsistent cooked? Currently we mute such errors and trust raw, but in this case either the raw or cooked could be incorrect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

should we throw if user provided inconsistent cooked?

I'm not sure.

Currently we mute such errors and trust raw, but in this case either the raw or cooked could be incorrect.

Can you give an example?
I test that fn \u is legal and \u is not.
Looks like it should be checked in TemplateLiteral.
Then we can do it later.😂

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For example, when user provided both raw and cooked:

t.templateElement({
  raw: "face",
  cooked: "faec" // which one is correct? face or faec?
})

The current main behaviour is leave it as-is. The PR behaviour is to ignore cooked and reassign cooked to be raw. Now thanks to unraw we know that this is not a valid template element. Should we throw a validation error in this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Make this a behavior in babel8?
But I don't know if jest snap supports this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now we ignore the cooked passed in by the user.
As for raw exceptions I think it can be done in the future.

@nicolo-ribaudo

Copy link
Copy Markdown
Member

We already have the logic to "unraw" template literals in our parser, I wonder if we could extract it from there to have a single implementation.

@liuxingbaoyu

Copy link
Copy Markdown
Member Author

@nicolo-ribaudo
The logic in parser is quite coupled, and keeping a single implementation seems difficult to do.
Actually I also tried to extract the code from parser at first, but didn't find it. 😓
I tried again just now and found it, but it looks like it's hard to even copy a copy of the code and modify it.

@liuxingbaoyu

Copy link
Copy Markdown
Member Author

@nicolo-ribaudo
I'm trying helper-string-parser and I'm having a little problem.
This way unterminatedCalled is true when raw is any value.

readStringContents("template", raw, 0, 0, 0, {
              unterminated() {
                unterminatedCalled = true;
              },
              strictNumericEscape: error,
              invalidEscapeSequence: error,
              numericSeparatorInEscapeSequence: error,
              unexpectedNumericSeparator: error,
              invalidDigit: error,
              invalidCodePoint: error,
            })

@nicolo-ribaudo

Copy link
Copy Markdown
Member

I think it's not true when raw is

abc`def

(which is actually an invalid raw, so we can throw if unterminatedCalled remains false)

@liuxingbaoyu

Copy link
Copy Markdown
Member Author

image

I'm not quite sure how I should use it.😕

@nicolo-ribaudo

Copy link
Copy Markdown
Member

Try this:

let unterminatedCalled = false;
let str, containsInvalid;
try {
  const error = () => {
    throw new Error();
  };
  ({ str, containsInvalid } = readStringContents(
    "template",
    "abc\\u{1000_0000}",
    0,
    0,
    0,
    {
      unterminated() {
        unterminatedCalled = true;
      },
      strictNumericEscape: error,
      invalidEscapeSequence: error,
      numericSeparatorInEscapeSequence: error,
      unexpectedNumericSeparator: error,
      invalidDigit: error,
      invalidCodePoint: error,
    }
  ));
} catch {
  // TODO: When https://github.com/babel/babel/issues/14775 is fixed
  // we can remove the try/catch block.
  unterminatedCalled = true;
  containsInvalid = true;
}

if (!unterminatedCalled) throw new Error("Invalid raw");

cooked = containsInvalid ? null : str;

some tests:

"abcdef"; // ok
"abc\u{123}def"; // ok
"abc\u{}def"; // null cooked
"abc\u{10000_0000}def"; // null cooked
"abc`def"; // Invalid raw, error
"abc`"; // Invalid raw, error
"`abc"; // Invalid raw, error
"abc${"; // Invalid raw, error
"${abc"; // Invalid raw, error
"abc\\`"; // ok

@liuxingbaoyu

liuxingbaoyu commented Jul 20, 2022

Copy link
Copy Markdown
Member Author

image

It's weird.
Is str being '' expected?

@nicolo-ribaudo

nicolo-ribaudo commented Jul 20, 2022

Copy link
Copy Markdown
Member

Whops sorry, it's a bug in helper-string-parser (that does not affect @babel/parser): can you add out += input.slice(chunkStart, pos); after the first error.unterminated?

@liuxingbaoyu liuxingbaoyu force-pushed the templateElement-cooked branch from 95bb3bb to 6f0d90f Compare July 20, 2022 17:50
@liuxingbaoyu

Copy link
Copy Markdown
Member Author

Now it works fine!

@nicolo-ribaudo nicolo-ribaudo left a comment

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.

Thank you! I just added two new tests.

As a Babel 8 follow-up, we could always try to generate .cooked and throw if the generated version is different from the one passed as an argument.

@liuxingbaoyu liuxingbaoyu changed the title feat: Automatically generate cooked for templateElement. feat: Automatically generate cooked for templateElement Jul 21, 2022
@liuxingbaoyu liuxingbaoyu merged commit 32d4f6e into babel:main Jul 21, 2022
@victor-wm

Copy link
Copy Markdown

@liuxingbaoyu do you know when this is going to be released?

@liuxingbaoyu

Copy link
Copy Markdown
Member Author

I'm not sure, generally released by @nicolo-ribaudo and @JLHwung .

@nicolo-ribaudo

Copy link
Copy Markdown
Member

I can release later today!

@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 Nov 1, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 1, 2022
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 PR: New Feature 🚀 A type of pull request used for our changelog categories

Projects

None yet

5 participants