Support resolveJsonModule in new module modes #46434
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
There was a problem hiding this comment.
I think // @ts-ignore is fine for suppressing this, but want to get @DanielRosenwasser’s input before merging
|
As long as this only affects node12 and nodenext this should be fine. |
|
Yep - the error should only be issued under the new module modes (as they're the only time file/import modes aren't |
|
i am wondering a bit about |
|
All diagnostics in TS are errors. We don't issue non-errors. Plus, it legitimately doesn't work at runtime under default configuration. |
| @@ -2637,6 +2637,14 @@ namespace ts { | |||
| return usageMode === ModuleKind.ESNext && targetMode === ModuleKind.CommonJS; | |||
| } | |||
|
|
|||
| function isOnlyImportedAsDefault(usage: Expression) { | |||
There was a problem hiding this comment.
should this name include Json somewhere, like isJsonOnlyImportedAsDefault? I thought it might be evident from its usage, but it appears to be used in a place with all kinds of imports.
I also don't understand what 'Only' means here.
There was a problem hiding this comment.
Only means in this case that for example
import { version } from './package.json'does not work only the default works so
import pkg from './package.json'
const { version } = pkg;only means that no destructuring style named imports are supported.
| if (hasDefaultOnly && type && !isErrorType(type)) { | ||
| const synthType = type as SyntheticDefaultModuleType; | ||
| if (!synthType.defaultOnlyType) { | ||
| const type = createDefaultPropertyWrapperForModule(symbol, originalSymbol); |
There was a problem hiding this comment.
not related to this PR, but: is this default-wrapper behaviour likely to change as importing JSON stops being experimental?
There was a problem hiding this comment.
@sandersn nope it should not stop as the behavior will stay there are currently no plans to change that inside nodejs so only default imports are supported for json type imports as the json is always a whole document there is no partial json parser.
* Support resolveJsonModule in new module modes * Formatting feedback
Fixes #46362
The bulk of this change is modifying the behavior for esm mode json imports, which no longer have named imports available to them. Such imports are still flagged by a
JSON imports are experimental in ES module mode importsdiagnostic, however, since esm json imports are experimental. I don't know if we'd want another flag to suppress this error or not, but I think it's an important error to have, since esm mode json imports are still up in the air as to weather they require import assertions or not, and so may need adjustment in the future. In any case, with the behavior updated, the error can always be//@ts-ignore'd if you're using the--experimental-json-modulesnode flag.