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

Deprecate ScriptElementKind.jsxAttribute #47414

Merged
merged 1 commit into from Jan 18, 2022

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jan 12, 2022

SymbolKind.getSymbolKind's handling of JSX attributes appears to be pretty broken (and has been for a while), but doesn't seem to have been noticed (or at least reported) by anyone until the JSX attribute snippet feature started using it as a way to detect that an attribute was being completed (#47090, #47280).

For example:

image

Rather than attempt to fix it by introducing contextToken into the mix (which I attempted in a revision of #47096), we decided that it was inconsistent that this function (which mainly powers the parenthesized string at the beginning of tooltips) uses syntactic context at all, as opposed to just describing the symbol.

This PR eliminates jsxAttribute entirely, which means that JSX attributes, tag names, etc, will all show as variables/properties/etc, the same as they would be shown when used in other places (since JSX attributes are just properties on a parameter object), eliminating the broken logic used to detect attributes.

Now:

image

image

image

image

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 12, 2022
@jakebailey jakebailey marked this pull request as ready for review January 14, 2022 07:11
@jakebailey jakebailey added this to Not started in PR Backlog via automation Jan 14, 2022
@jakebailey jakebailey moved this from Not started to Waiting on reviewers in PR Backlog Jan 14, 2022
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.

I wasn't part of the decision to deprecate jsxAttribute, but I think it's a good idea.

PR Backlog automation moved this from Waiting on reviewers to Needs merge Jan 18, 2022
@jakebailey jakebailey merged commit 2635102 into microsoft:main Jan 18, 2022
PR Backlog automation moved this from Needs merge to Done Jan 18, 2022
@jakebailey jakebailey deleted the remove-jsx-attribute branch January 18, 2022 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants