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

Adjust scanning of keywordy jsx namespace names, add grammar error for jsx dotted names containing namespace names #43104

Merged
merged 2 commits into from Mar 22, 2021

Conversation

weswigham
Copy link
Member

Fixes #42963

The JSX spec doesn't visibly forbid namespace names where a component of the namespace is a keyword, so those are adjusted to parse as Identifiers (so they then typecheck), and I've also added a grammar error on a:b.c and the like.

…r jsx dotted names containing namespace names
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 5, 2021
@sandersn sandersn added this to Not started in PR Backlog Mar 16, 2021
PR Backlog automation moved this from Not started to Needs merge Mar 19, 2021
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

One nit that's more of an observation in case you want to change it.

}
}

function checkGrammarJsxNestedIdentifier(name: MemberName | ThisExpression) {
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't have a strong reason to be nested, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly just to avoid pollution in the containing scope, since it's only needed here. Since it doesn't close over anything, it should be just as effective as a function in the surrounding scope.

@weswigham weswigham merged commit 9a256a1 into microsoft:master Mar 22, 2021
PR Backlog automation moved this from Needs merge to Done Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Jsx namespaced name and attributes
3 participants