add related error span for default exports #25396
Conversation
Kingwl
force-pushed
the
relatedExportDefault
branch
from
50a3c61 to
656fbc0
Compare
| @@ -7,6 +7,7 @@ tests/cases/conformance/es6/modules/m1.ts(10,16): error TS2528: A module cannot | |||
| export default class foo { | |||
| ~~~ | |||
| !!! error TS2528: A module cannot have multiple default exports. | |||
| !!! related TS2730 tests/cases/conformance/es6/modules/m1.ts:1:22: This export conflicts with the first. | |||
There was a problem hiding this comment.
that is because function bind first with bindEachFunctionsFirst,
seems expect behavior
Kingwl
changed the title
src/compiler/binder.ts
Outdated
| const declarationName = getNameOfDeclaration(node) || node; | ||
| const diag = createDiagnosticForNode(declarationName, message, getDisplayName(node)) | ||
| file.bindDiagnostics.push( | ||
| multipleDefaultExports ? addRelatedInfo(diag, createDiagnosticForNode(declarationName, Diagnostics.This_export_conflicts_with_the_first)) : diag |
There was a problem hiding this comment.
Both the related info and the original diagnostic have the same span here - seems wrong. The related span should be the first declaration, not this diagnostic's declaration.
src/compiler/diagnosticMessages.json
Outdated
| @@ -2409,6 +2409,10 @@ | |||
| "category": "Error", | |||
| "code": 2729 | |||
| }, | |||
| "This export conflicts with the first.": { | |||
There was a problem hiding this comment.
I know the original issue said the message should be This export conflicts with the first. - but that would be if we issued only one diagnostic (on the first default export), and then just added a bunch of related spans to it. In the case of what we're doing here (which I think is OK), we're keeping all the diagnostics, but adding related spans on them. In that case, I think the message should be The first export default is here. for the non-first ones and Another export default is here (for the first) ... and here. (for the rest) for the first one.
So you get an error like
!!! error TS2528: A module cannot have multiple default exports.
!!! related TS2730 <first decl location> : The first export default is here
and
!!! error TS2528: A module cannot have multiple default exports.
!!! related TS2730 <other decl location 1> : Another export default is here
!!! related TS2730 <other decl location 2> : and here
There was a problem hiding this comment.
cc @DanielRosenwasser since you had the original issue
There was a problem hiding this comment.
and here😂 have we already get this message?
Kingwl
force-pushed
the
relatedExportDefault
branch
from
656fbc0 to
2ffa94c
Compare
Kingwl
force-pushed
the
relatedExportDefault
branch
from
760bcff to
c261ca2
Compare
|
@weswigham can u please review and merge |
|
@Kingwl can you add a test with >2 default exports so we can see a test with the |
|
@weswigham |
|
⬆️ |
Fixes #25032