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

Add raw to Identifier? #291

Open
fisker opened this issue Oct 29, 2022 · 6 comments
Open

Add raw to Identifier? #291

fisker opened this issue Oct 29, 2022 · 6 comments

Comments

@fisker
Copy link

fisker commented Oct 29, 2022

In some cases, it's unsafe to unescape Identifier.

Because of typeAnnotation, it's also hard to get the raw text. Maybe we can add unescaped text to Identifier.raw?

@bradzacher
Copy link

In some cases, it's unsafe to unescape Identifier.

Have you got some examples?

@fisker
Copy link
Author

fisker commented Oct 29, 2022

I'm working on adding support for "Explicit Resource Management" proposal in Prettier, there are two tests added in babel PR.

for await (using \u006ff of of);
for (using o\u0066 of of);

They are valid, but if we format them to of, they will be invalid.

@JLHwung

@JLHwung
Copy link
Contributor

JLHwung commented Oct 30, 2022

Yes, unescaping an identifier is not always safe. Here is a similar test in current ES:

for (async of []); // invalid
for (\u0061sync of []); // valid

However, from ESTree's perspective, I don't think we should expand the AST. Instead it should be a generator/printer rule that identifier names are not always safe to unescape. First, \u0061sync, asyn\u0063, etc. share the same semantics (abstract syntax), the AST doesn't care which character is escaped. Second, the AST gives no guarantee that a generator will generate correct code when printing an identifier name. The generator should implement certain rules that an identifier must be escaped.

In another hand, if ESTree were a CST standard, we should definitely record that which characters have been escaped.

@fisker
Copy link
Author

fisker commented Oct 30, 2022

Thanks for sharing more examples.

However, from ESTree's perspective, I don't think we should expand the AST.

Yes, it's like Property.shorthand and Literal.raw, only useful for generator. But still worth to try.

Maybe I should ask parser to extend Indentifier instead?

I just noticed that Literal.raw is not also included in the spec #14, so I guess there is no chance to approve this one at all.

@nzakas
Copy link
Contributor

nzakas commented Nov 1, 2022

Yes, as a reminder, ESTree is not a spec that parsers must adhere to, but rather a loose agreement between Babel, ESLint, and Acorn around what to expect in an AST.

The problem with adding new properties to existing objects is that there will never be any guarantee that those new properties will be present, so you can’t rely on that data being present either way.

For this case, I think it’s better to work with specific parsers you want to use to add in additional info.

@conartist6
Copy link

Here's what I think it would look like if there were a shared CST format:
https://gist.github.com/conartist6/b548adbf385d1ed6e226b25b67f8b46b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants