Skip to content
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

Module Resolution: imports with JSON assertion should not need flag when module is esnext #54929

Open
tpluscode opened this issue Jul 8, 2023 · 5 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@tpluscode
Copy link

Demo Repo

https://github.com/tminuscode/resolve-json-module

Which of the following problems are you reporting?

The module specifier resolves at runtime, but not at build time

Demonstrate the defect described above with a code sample.

import htmlTags from 'html-tags/html-tags.json' assert { type: 'json' }

const keys = Object.keys(htmlTags)

Run tsc --showConfig and paste its output here

{
"compilerOptions": {
"target": "es2020",
"module": "esnext",
"moduleResolution": "node16"
},
"files": [
"./index.ts"
]
}

Run tsc --traceResolution and paste its output here

https://pastebin.com/GzfXrXi4

Sorry, content too long

Paste the package.json of the importing module, if it exists

{
"name": "resolve-json-module",
"dependencies": {
"html-tags": "^3.3.1",
"typescript": "^5.1.6"
}
}

Paste the package.json of the target module, if it exists

{
"name": "html-tags",
"version": "3.3.1",
"description": "List of standard HTML tags",
"license": "MIT",
"repository": "sindresorhus/html-tags",
"funding": "https://github.com/sponsors/sindresorhus",
"author": {
"name": "Sindre Sorhus",
"email": "sindresorhus@gmail.com",
"url": "https://sindresorhus.com"
},
"engines": {
"node": ">=8"
},
"scripts": {
"test": "xo && ava && tsd"
},
"files": [
"index.js",
"index.d.ts",
"void.js",
"void.d.ts",
"html-tags.json",
"html-tags-void.json"
],
"keywords": [
"html",
"html5",
"tags",
"elements",
"list",
"whatwg",
"w3c",
"void",
"self-closing"
],
"devDependencies": {
"ava": "^1.4.1",
"tsd": "^0.7.2",
"xo": "^0.24.0"
},
"xo": {
"rules": {
"import/extensions": "off"
}
}
}

Any other comments can go here

I wan to use the latest ES feature of asserted imports to directly load a JSON from the package html-tags. The feature requires targeting esnext and I expected it to be enough, given the no compilation actually happens - the syntax is exactly the same in TS and JS

However, to actually make tsc happy, it is necessary to add --resolveJsonModule --allowSyntheticDefaultImports. Without these two, I'm getting these errors

error TS2732: Cannot find module 'html-tags/html-tags.json'. Consider using '--resolveJsonModule' to import module with '.json' extension.

resolveJsonModule seems completely redundant because importing JSON is supported per assert { type: 'json' }

error TS1259: Module '"/Volumes/Home/projects/temp/resolve-json-module/node_modules/html-tags/html-tags"' can only be default-imported using the 'allowSyntheticDefaultImports' flag
[...]
This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

I'm not sure about this. Isn't default-import the only way to import a JSON file?

@Jamesernator
Copy link

Jamesernator commented Jul 9, 2023

"module": "esnext",
"moduleResolution": "node16"

Both of these should be node16 (or nodenext), mixing these has never really worked properly and in fact in TypeScript 5.2 mixing won't be allowed because it doesn't work.

You won't need allowSyntheticDefaultImports if you do so, however you'll still need resolveJsonModule. You'll also need to add "type": "module" to your package.json if you want import in .js files.

it is necessary to add --resolveJsonModule

Any command line options can be put into tsconfig.json instead, so you can just have:

// tsconfig.json
{
  "compilerOptions": {
    "target": "ES2020",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "resolveJsonModule": true
  }
}

because importing JSON is supported per assert { type: 'json' }

At present JSON modules are still experimental in Node and Browsers, in fact if you're using import assertions without transpilation this might break in future as the proposal is being changed to replace assert with with (in addition to some semantic changes).

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 10, 2023

However, to actually make tsc happy, it is necessary to add --resolveJsonModule --allowSyntheticDefaultImports. Without these two, I'm getting these errors

ASDI addressed above; I don't understand why you're saying it's a bug that you have to specify resolveJsonModule in order to resolve JSON modules. Can you expand on that?

@tpluscode
Copy link
Author

Both of these should be node16 (or nodenext)

Thank you, I did not know Node(16|Next) were also allowed for module. This answers my question.

I don't understand why you're saying it's a bug that you have to specify resolveJsonModule in order to resolve JSON modules. Can you expand on that?

@RyanCavanaugh because, albeit warning as being an experimental feature, import assertions work by default since node 16. Why hide it behind a flag in TS (given appropriate module(Resolution) settings).

Granted, as said above, this feature may change. So do new TS5 decorators, but they do not require a flag. They actually only work with the previous experimentalDecorators flag disabled.


I suppose you'd close this issue if you think that the RJM flag remark is not really a bug.

@Jamesernator
Copy link

Granted, as said above, this feature may change. So do new TS5 decorators, but they do not require a flag.

One particular difference is that decorators can be transpiled away by choosing a different target, however import assertions cannot be. Using import assertions requires knowing that the host will actually support the feature, it cannot be transpiled away.

To be honest though I do think it would make sense for "module": "Node..." to imply resolveJsonModule once the feature is stable.

import assertions work by default since node 16.

If you're using import assertions in current Node then you are currently getting the following warnings printed:

(node:14898) ExperimentalWarning: Import assertions are not a stable feature of the JavaScript language. Avoid relying on their current behavior and syntax as those might change in a future version of Node.js.
(node:14898) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time

These warnings are serious, as mentioned if it's sufficiently web compatible the assert keyword is going to be removed entirely in browsers, in which case Node will likely follow suit.

@RyanCavanaugh
Copy link
Member

The current behavior is that import assertions don't change the behavior of module resolution, which makes it more consistent. We could make it less consistent and say that certain assertions change certain behavior, but I'd like to see a proposal what sorts of assertions should cause which sorts of changes

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants