Skip to content

AST change proposal: Improve TSModuleDeclaration #6440

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.

Sign up for GitHub

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

Jump to bottom
Closed
4 tasks done
fisker opened this issue Feb 8, 2023 · 7 comments · Fixed by #6443
Closed
4 tasks done

AST change proposal: Improve TSModuleDeclaration #6440

fisker opened this issue Feb 8, 2023 · 7 comments · Fixed by #6443
Labels
duplicate This issue or pull request already exists

Comments

@fisker
Copy link
Contributor

fisker commented Feb 8, 2023 •
edited
Loading

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

ast-spec

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Currently

module a.b {}
^^^^^^^^^^^^^ TSModuleDeclaration(a)
       ^      TSModuleDeclaration(a).id
         ^^^^ TSModuleDeclaration(b)
         ^    TSModuleDeclaration(b).id
           ^^ TSModuleBlock

Can we change it to

module a.b {}
^^^^^^^^^^^^^ TSModuleDeclaration
       ^^^    TSModuleDeclaration.id
           ^^ TSModuleBlock

?

Another problem is we parse the following code as the same ast

namespace a {}
module a {}

Prettier can't distinguish them without the original code. Failure detection

Maybe add TSNamespaceDeclaration or TSModuleDeclaration.namespace?

Fail

N/a

Pass

N/a

Additional Info

No response
@fisker fisker added enhancement New feature or request triage Waiting for team members to take a look labels Feb 8, 2023
@bradzacher
Copy link
Member

Duplicate of #4966

We'll be fixing this in the next major!

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2023
@bradzacher bradzacher added duplicate This issue or pull request already exists and removed enhancement New feature or request triage Waiting for team members to take a look labels Feb 8, 2023
@fisker
Copy link
Contributor Author

fisker commented Feb 8, 2023

Sorry, didn't find it, must use the wrong keyword.

@bradzacher
Copy link
Member

Another problem is we parse the following code as the same ast

That's cos they're exactly the same from the POV of TS.
Both emit a ts.ModuleDeclaration node and there are no special properties to identify one case or the other.

I think maybe node.flags & ts.NodeFlags.Namespace might identify the namespace case? It would require some research to figure out all of the cases to determine if it does.

The other option is that prettier could inspect the TS node for the specific keyword?
Maybe we could add that to the AST? 🤷

@fisker
Copy link
Contributor Author

fisker commented Feb 8, 2023

I think maybe node.flags & ts.NodeFlags.Namespace might identify the namespace case? It would require some research to figure out all of the cases to determine if it does.

👍

The other option is that prettier could inspect the TS node for the specific keyword?

Maybe already in tokens? We are not using any token now.

@bradzacher
Copy link
Member

yeah it's in the tokens we emit:
image

It's also in the ts-ast as well as a child of the module declaration node.

@fisker
Copy link
Contributor Author

fisker commented Feb 8, 2023

How do you think the idea

Maybe add TSNamespaceDeclaration or TSModuleDeclaration.namespace?

@fisker
Copy link
Contributor Author

fisker commented Feb 8, 2023

Or TSModuleDeclaration.kind = "namespace" | "module"?

@bradzacher bradzacher mentioned this issue Feb 9, 2023
Merged
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
duplicate This issue or pull request already exists
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add .kind to TSModuleDeclaration typescript-eslint/typescript-eslint
3 participants
@fisker @JoshuaKGoldberg @bradzacher