Skip to content

Better error message for mistaken import #40

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
merged 1 commit into from
Oct 1, 2019

Conversation

prime-optimus
Copy link

Issue number of the reported bug or feature request: microsoft#13601

@mheiber
Copy link

mheiber commented Sep 30, 2019

reviewing

Copy link

@mheiber mheiber left a comment

Choose a reason for hiding this comment

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

a few minor stylistic changes, but other than that looks good to me!

@@ -2297,7 +2308,17 @@ namespace ts {
}
}
else {
error(name, Diagnostics.Module_0_has_no_exported_member_1, moduleName, declarationName);
if (moduleSymbol.exports && moduleSymbol.exports.has("default" as __String)) {
Copy link

Choose a reason for hiding this comment

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

Judging by other code in the checker, I think this could be more idiomatic.

  • can you try .has(InternalSymbolName.Default) instead of a hard-coded string? Then it's more consistent with the surrounding code and we don't need a cast.

@@ -1,4 +1,4 @@
tests/cases/compiler/service.ts(1,8): error TS1192: Module '"tests/cases/compiler/db"' has no default export.
tests/cases/compiler/service.ts(1,8): error TS6222: Module '"tests/cases/compiler/db"' has no default export. Did you mean to use 'import {db} from "tests/cases/compiler/db"' instead?
Copy link

Choose a reason for hiding this comment

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

I think it's more idiomatic to see a space around the identifier in the curlies: import { db } from ....
Basing this on the formatting for destructuring assignment in the TS codebase, as well as how prettier formats imports, to the best of my knowledge.

@prime-optimus
Copy link
Author

@mheiber addressed your review comments.

@mheiber mheiber merged commit e0cda78 into bloomberg:13624 Oct 1, 2019
mheiber pushed a commit that referenced this pull request Oct 1, 2019
Issue number of the reported bug or feature request: microsoft#13601
@mheiber
Copy link

mheiber commented Oct 1, 2019

it's live! microsoft#33706

@prime-optimus prime-optimus deleted the test branch June 2, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants