Skip to content

Support import completion for merged declaration exports #24539

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

Merged
1 commit merged into from
Jun 1, 2018

Conversation

dfreeman
Copy link
Contributor

Fixes #24535

I'm frankly not familiar enough with the codebase to be positive that this is a correct fix, but
this assertion was failing because the result of getAllReExportingModules was empty. The included test failed without this change, and the full suite passes locally with it.

@msftclas
Copy link

msftclas commented May 31, 2018

CLA assistant check
All CLA requirements met.

@mhegazy mhegazy requested a review from a user May 31, 2018 20:53
@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2018

@Andy-MS can you please review

@ghost
Copy link

ghost commented May 31, 2018

I think the best fix would be in completions.ts where currently:

const exportedSymbol = skipAlias(symbol.exportSymbol || symbol, checker);

should be:

const exportedSymbol = checker.getMergedSymbol(skipAlias(symbol.exportSymbol || symbol, checker));

Then importFixes has the correct symbol it needs to compare against and doesn't have to work around.

@dfreeman dfreeman force-pushed the auto-import-merged-declaration branch from 83c257d to 172a42b Compare June 1, 2018 16:15
@dfreeman
Copy link
Contributor Author

dfreeman commented Jun 1, 2018

@Andy-MS Updated. Thanks for the pointer—that makes much more sense 🙂

@ghost
Copy link

ghost commented Jun 1, 2018

Thanks!

@ghost ghost merged commit 4a36ee0 into microsoft:master Jun 1, 2018
@dfreeman dfreeman deleted the auto-import-merged-declaration branch June 1, 2018 17:32
@microsoft microsoft locked and limited conversation to collaborators Aug 2, 2018
This pull request was closed.
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.

Auto-import fails for default exports that are the result of declaration merging
3 participants