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 directives field to TSModuleDeclaration? #3564

Closed
overlookmotel opened this issue Jun 6, 2024 · 2 comments · Fixed by #3830
Closed

Add directives field to TSModuleDeclaration? #3564

overlookmotel opened this issue Jun 6, 2024 · 2 comments · Fixed by #3830
Assignees
Labels
A-ast Area - AST A-parser Area - Parser

Comments

@overlookmotel
Copy link
Contributor

From #3532, it seems like this is legal syntax and 'use strict'; should be interpreted as a Directive:

namespace Foo {
    'use strict';
    export const bar = 123;
}

Should we add a directives: Vec<'a, Directive<'a>> field to TSModuleDeclaration and fill it in the parser, using same logic as for functions?

@overlookmotel overlookmotel added the A-ast Area - AST label Jun 6, 2024
@Boshen
Copy link
Member

Boshen commented Jun 6, 2024

Babel parser hands out Directive nodes. We can do this.

@Boshen Boshen added the A-parser Area - Parser label Jun 6, 2024
@overlookmotel
Copy link
Contributor Author

overlookmotel commented Jun 6, 2024

Babel doesn't use Directives, as far as I can see, and nor does typescript-eslint or SWC:

AST explorer - Babel
AST explorer - typescript-eslint
AST explorer - SWC

They all produce a StringLiteral. But I think they're all doing it wrong!

We could just ignore this, and leave it as is. It's not at all a common case. But it will cause semantic to be incorrect. This should be an error (even if alwaysStrict: false in TS config), but it won't be:

namespace Foo {
    'use strict';
    let arguments;
}

Typescript gets this right: TS playground

NB: If we do this, we'd need to add a custom Serialize impl to get ESTree-compatible JSON AST output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-parser Area - Parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants