Skip to content

Fix #20805#21755

Merged
mhegazy merged 7 commits into
masterfrom
Fix20805
Feb 8, 2018
Merged

Fix #20805#21755
mhegazy merged 7 commits into
masterfrom
Fix20805

Conversation

@mhegazy
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy commented Feb 8, 2018

React fragments are one of the cases that were not handled correctly. we assume all umd imports are identifiers, and fail otherwise. this change:

  • adds support for react fragements
  • changes the flow to resemble checkJsxOpeningLikeElementOrOpeningFragment logic that reports the error in the first place
  • Adds a check for Exception from convertToImportCodeFixContext #21607 to avoid crashing if the token is not an identifier

Fixes #20805
Fixes #21767
Mitigates #21607

@mhegazy mhegazy requested a review from a user February 8, 2018 07:46
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Don't think this is a fix of #21607 though, as that issue would apply to all code fixes.

Comment thread src/services/codefixes/importFixes.ts Outdated
const symbol = checker.getAliasedSymbol(umdSymbol);
if (symbol) {
return getCodeActionsForImport([{ moduleSymbol: symbol, importKind: getUmdImportKind(context.program.getCompilerOptions()) }],
convertToImportCodeFixContext(context, isIdentifier(token) ? token : undefined, umdSymbol.name));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

context.symbolToken is currently typed as Node | undefined and we check if it's an identifier when using it. So there is currently redundant checking. We could change its type to Identifier | undefined and remove the check at its use, or remove the check here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

// This will always be an Identifier, since the diagnostics we fix only fail on identifiers.
const { sourceFile, span, program, cancellationToken } = context;
const checker = program.getTypeChecker();
const symbolToken = getTokenAtPosition(sourceFile, span.start, /*includeJsDocComment*/ false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like this is the same in both UMD and non-UMD cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes. but i wanted to defer the call to convertContext untill i had the name.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant