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: Regression in JSON import using Node16 resolution #60589

Closed
raykyri opened this issue Nov 25, 2024 · 16 comments · Fixed by #60673
Closed

Module resolution: Regression in JSON import using Node16 resolution #60589

raykyri opened this issue Nov 25, 2024 · 16 comments · Fixed by #60673
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@raykyri
Copy link

raykyri commented Nov 25, 2024

Demo Repo

canvasxyz/canvas#405

Which of the following problems are you reporting?

Something else more complicated which I'll explain in more detail

Demonstrate the defect described above with a code sample.

import * as pkg from "../package.json"

Run tsc --showConfig and paste its output here

> cd examples/chat-explorer
> npx tsc --showConfig
{
    "compilerOptions": {
        "composite": true,
        "strict": true,
        "declaration": true,
        "module": "node16",
        "target": "es2020",
        "moduleResolution": "node16",
        "allowSyntheticDefaultImports": true,
        "useDefineForClassFields": true,
        "skipLibCheck": true,
        "rootDir": "./src",
        "outDir": "./lib",
        "lib": [
            "es2020",
            "dom",
            "dom.iterable"
        ],
        "jsx": "react-jsx",
        "noUnusedLocals": true,
        "noFallthroughCasesInSwitch": true,
        "resolveJsonModule": true,
        "moduleDetection": "force",
        "esModuleInterop": true,
        "resolvePackageJsonExports": true,
        "resolvePackageJsonImports": true,
        "incremental": true,
        "noImplicitAny": true,
        "noImplicitThis": true,
        "strictNullChecks": true,
        "strictFunctionTypes": true,
        "strictBindCallApply": true,
        "strictPropertyInitialization": true,
        "strictBuiltinIteratorReturn": true,
        "alwaysStrict": true,
        "useUnknownInCatchVariables": true
    },
    "references": [
        {
            "path": "../../packages/core"
        },
        {
            "path": "../../packages/chain-atp"
        },
        {
            "path": "../../packages/chain-cosmos"
        },
        {
            "path": "../../packages/chain-ethereum"
        },
        {
            "path": "../../packages/chain-ethereum-viem"
        },
        {
            "path": "../../packages/chain-solana"
        },
        {
            "path": "../../packages/chain-substrate"
        },
        {
            "path": "../../packages/signatures"
        }
    ],
    "files": [
        "./src/ActionsTable.tsx",
        "./src/HomePage.tsx",
        "./src/NetworkPlot.tsx",
        "./src/SessionsTable.tsx",
        "./src/main.tsx",
        "./src/useCursorStack.ts",
        "./src/utils.ts",
        "./src/vite-env.d.ts",
        "./src/components/DidPopover.tsx",
        "./src/components/Navbar.tsx",
        "./src/components/PaginationButton.tsx"
    ],
    "include": [
        "src"
    ],
    "exclude": [
        "/Users/selkie/Development/canvas/examples/chat-explorer/lib"
    ]
}

Run tsc --traceResolution and paste its output here

File '/Users/selkie/Development/canvas/examples/chat-explorer/src/package.json' does not exist.
Found 'package.json' at '/Users/selkie/Development/canvas/examples/chat-explorer/package.json'.
======== Resolving module 'react/jsx-runtime' from '/Users/selkie/Development/canvas/examples/chat-explorer/src/ActionsTable.tsx'. ========
Explicitly specified module resolution kind: 'Node16'.
Resolving in ESM mode with conditions 'import', 'types', 'node'.
File '/Users/selkie/Development/canvas/examples/chat-explorer/src/package.json' does not exist according to earlier cached lookups.
File '/Users/selkie/Development/canvas/examples/chat-explorer/package.json' exists according to earlier cached lookups.
Loading module 'react/jsx-runtime' from 'node_modules' folder, target file types: TypeScript, JavaScript, Declaration, JSON.
Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.
Directory '/Users/selkie/Development/canvas/examples/chat-explorer/src/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/selkie/Development/canvas/examples/node_modules' does not exist, skipping all lookups in it.
Found 'package.json' at '/Users/selkie/Development/canvas/node_modules/react/package.json'.
Using 'exports' subpath './jsx-runtime' with target './jsx-runtime.js'.
File name '/Users/selkie/Development/canvas/node_modules/react/jsx-runtime.js' has a '.js' extension - stripping it.
File '/Users/selkie/Development/canvas/node_modules/react/jsx-runtime.ts' does not exist.
File '/Users/selkie/Development/canvas/node_modules/react/jsx-runtime.tsx' does not exist.
File '/Users/selkie/Development/canvas/node_modules/react/jsx-runtime.d.ts' does not exist.
Export specifier './jsx-runtime' does not exist in package.json scope at path '/Users/selkie/Development/canvas/node_modules/react'.
Found 'package.json' at '/Users/selkie/Development/canvas/node_modules/@types/react/package.json'.
Entering conditional exports.
Saw non-matching condition 'types@<=5.0'.
Matched 'exports' condition 'types'.
Entering conditional exports.
Matched 'exports' condition 'default'.
Using 'exports' subpath './jsx-runtime' with target './jsx-runtime.d.ts'.
File '/Users/selkie/Development/canvas/node_modules/@types/react/jsx-runtime.d.ts' exists - use it as a name resolution result.
'package.json' does not have a 'peerDependencies' field.
Resolved under condition 'default'.
Exiting conditional exports.
Resolved under condition 'types'.
Exiting conditional exports.
Resolving real path for '/Users/selkie/Development/canvas/node_modules/@types/react/jsx-runtime.d.ts', result '/Users/selkie/Development/canvas/node_modules/@types/react/jsx-runtime.d.ts'.
======== Module name 'react/jsx-runtime' was successfully resolved to '/Users/selkie/Development/canvas/node_modules/@types/react/jsx-runtime.d.ts' with Package ID '@types/react/jsx-runtime.d.ts@18.3.9'. ========
======== Resolving module 'swr' from '/Users/selkie/Development/canvas/examples/chat-explorer/src/ActionsTable.tsx'. ========
Explicitly specified module resolution kind: 'Node16'.
Resolving in ESM mode with conditions 'import', 'types', 'node'.
File '/Users/selkie/Development/canvas/examples/chat-explorer/src/package.json' does not exist according to earlier cached lookups.
File '/Users/selkie/Development/canvas/examples/chat-explorer/package.json' exists according to earlier cached lookups.
Loading module 'swr' from 'node_modules' folder, target file types: TypeScript, JavaScript, Declaration, JSON.
Searching all ancestor node_modules directories for preferred extensions: TypeScript, Declaration.
Directory '/Users/selkie/Development/canvas/examples/chat-explorer/src/node_modules' does not exist, skipping all lookups in it.
Directory '/Users/selkie/Development/canvas/examples/node_modules' does not exist, skipping all lookups in it.
Found 'package.json' at '/Users/selkie/Development/canvas/node_modules/swr/package.json'.
Entering conditional exports.
Saw non-matching condition 'react-server'.
Matched 'exports' condition 'import'.
Entering conditional exports.
Matched 'exports' condition 'types'.
Using 'exports' subpath '.' with target './dist/core/index.d.mts'.
File '/Users/selkie/Development/canvas/node_modules/swr/dist/core/index.d.mts' exists - use it as a name resolution result.
'package.json' has a 'peerDependencies' field.
Resolving real path for '/Users/selkie/Development/canvas/node_modules/swr', result '/Users/selkie/Development/canvas/node_modules/swr'.
File '/Users/selkie/Development/canvas/node_modules/react/package.json' exists according to earlier cached lookups.
Found peerDependency 'react' with '18.3.1' version.
Resolved under condition 'types'.

// ...

======== Module name 'vite' was successfully resolved to '/Users/selkie/Development/canvas/node_modules/vite/index.d.cts' with Package ID 'vite/index.d.cts@5.4.11+@types/node@20.14.10+lightningcss@1.28.1+terser@5.27.0'. ========
src/HomePage.tsx:1:22 - error TS1543: Importing a JSON file into an ECMAScript module requires a 'type: "json"' import attribute when 'module' is set to 'Node16'.

1 import * as pkg from "../package.json"
                       ~~~~~~~~~~~~~~~~~


Found 1 error in src/HomePage.tsx:1

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

{
	"name": "@canvas-js/chat-explorer",
	"private": true,
	"type": "module",
	"version": "0.12.5",
	"scripts": {
		"dev": "vite",
		"build": "vite build",
		"lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0",
		"dev:server": "tsx --tsconfig server/tsconfig.json --watch server/server.ts",
		"start:server": "cross-env NODE_ENV=production node dist/server.js"
	},
	"dependencies": {
		"@canvas-js/chain-atp": "0.12.5",
		"@canvas-js/chain-cosmos": "0.12.5",
		"@canvas-js/chain-ethereum": "0.12.5",
		"@canvas-js/chain-solana": "0.12.5",
		"@canvas-js/chain-substrate": "0.12.5",
		"@canvas-js/core": "0.12.5",
		"@canvas-js/interfaces": "0.12.5",
		"@ipld/dag-json": "^10.2.2",
		"@radix-ui/themes": "^3.1.3",
		"@vitejs/plugin-react": "^4.3.1",
		"autoprefixer": "^10.4.20",
		"cors": "^2.8.5",
		"cross-env": "^7.0.3",
		"d3": "^7.9.0",
		"date-fns": "^3.6.0",
		"express": "^4.19.2",
		"express-ipld": "^0.0.1",
		"http-status-codes": "^2.3.0",
		"react": "^18.3.1",
		"react-dom": "^18.3.1",
		"react-router-dom": "^6.20.1",
		"swr": "^2.2.5",
		"vite": "^5.4.8"
	},
	"devDependencies": {
		"@types/cors": "^2.8.17",
		"@types/d3": "^7.4.3",
		"@types/express": "^4.17.21",
		"@types/node": "^18.11.5",
		"@types/react": "^18.3.3",
		"@types/react-dom": "^18.3.0"
	}
}

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

n/a

Any other comments can go here

A recent change that landed around 5.7.0 seems to break Node16 imports of .json files, because a type: json import attributed is required but also not allowed to be used: canvasxyz/canvas#404

It might be related to this issue: #59684

Which was addressed in this PR: #60019

It seems to produce new errors in these automated builds: https://github.com/search?q=repo%3Amicrosoft%2FTypeScript+%22named+imports+are+not+allowed%22&type=issues

@kirkwaiblinger
Copy link

kirkwaiblinger commented Nov 26, 2024

Concise repro:

// "module": "node16", "resolveJsonModule": true, in a *.mts file (does not apply to cjs output)

// TS error: Importing a JSON file into an ECMAScript module requires a 'type: "json"' import attribute when 'module' is set to 'Node16'.ts(1543)
import something1 from './someThing.json';

// TS error: Import attributes are only supported when the '--module' option is set to 'esnext', 'nodenext', or 'preserve'.ts(2823)
import something2 from './someThing.json' with { type: "json" };

(related, #60598)

@jakebailey
Copy link
Member

@andrewbranch

@jakebailey
Copy link
Member

The issue is pretty poorly formatted, so to restate, the problem is that we require an import attribute for JSON in Node16 resolution, but import attributes are not allowed in Node16 resolution?

@raykyri
Copy link
Author

raykyri commented Nov 26, 2024

That seems to be what's happening on our side, yes.

@andrewbranch andrewbranch self-assigned this Nov 26, 2024
@andrewbranch andrewbranch added the Bug A bug in TypeScript label Nov 26, 2024
@andrewbranch andrewbranch added this to the TypeScript 5.7.3 milestone Nov 26, 2024
@t3chguy
Copy link

t3chguy commented Nov 27, 2024

I am seeing the same thing, and even if I bump the module to nodenext that does not help as import type assertions are forbidden on import type statements

src/language-helper.ts:14:49 - error TS2822: Import assertions cannot be used with type-only imports or exports.

14 import type EN from "./i18n/strings/en_EN.json" assert { type: "json" };
src/language-helper.ts:14:21 - error TS1543: Importing a JSON file into an ECMAScript module requires a 'type: "json"' import attribute when 'module' is set to 'Node16'.

14 import type EN from "./i18n/strings/en_EN.json";

@andrewbranch
Copy link
Member

In Node.js 16, JSON files can’t be imported without an import assertion, and import attributes aren’t supported until 18 (I’m ignoring odd-numbered versions). It seems like it was a bug/oversight to allow JSON imports without assertions in --module node16, but I don’t think we want to suggest anyone use import assertions now. I guess we should just make #60019 have no effect on node16, even though node16 seems to have been incorrect all along. Node.js v16 is EOL now, so I think we should deprecate --module node16 and make a --module node18 that enforces use of import attributes for JSON imports (like nodenext does now).

@kirkwaiblinger
Copy link

@andrewbranch In principle, this resolution makes sense to me. However, I think the docs are problematic here.

I have it ingrained into my head, and I'm guessing that many users do as well, that that the *Next settings are more or less aliases of their latest concrete version.

The TS docs have this:

nodenext: Currently identical to node16, but will be a moving target reflecting the latest Node.js versions as Node.js’s module system evolves.

...and this:

node16 and nodenext are currently identical, with the exception that they imply different target option values. If Node.js makes significant changes to its module system in the future, node16 will be frozen while nodenext will be updated to reflect the new behavior.

It seems like nodenext is already a nontrivial departure from node16 in undocumented ways. My request as a user, having spent many hours tracking down issues of which this issue and #60598, are the root causes, and
not having thought to exhaustively check node16 vs nodenext in addition to all the other relevant settings, would be that

  1. the docs be quite clear going forward on all differences between the highest concrete version and the *.next versions of settings (including lib/target/module/modluleresolution ESNext/nodenext).
  2. concrete versions be pinned more frequently (like the proposed node18 in this issue) when nontrivial departures occur, so that users can actually use potentially-necessary features without relying on an unstable setting.

The reason I say this here, rather than immediately spinning off a new issue, is that I note that this is marked for TS 5.7.3, and I worry that subtly dropping module: Node18 in a patch version is not enough fanfare to break through to users who may be wasting hours not knowing they're running into a node16/node18/nodenext discrepancy. Furthermore, there may be other undocumented node16 vs nodenext discrepancies that would have to be included in a stable node18 module version. I don't know.

So I'm curious what the gameplan for actually fixing and communicating this issue out is.

@kirkwaiblinger
Copy link

kirkwaiblinger commented Dec 3, 2024

To be concrete, it seems to be that this can (and should) be done in a patch change, 👍

In Node.js 16, JSON files can’t be imported without an import assertion, and import attributes aren’t supported until 18 (I’m ignoring odd-numbered versions). It seems like it was a bug/oversight to allow JSON imports without assertions in --module node16, but I don’t think we want to suggest anyone use import assertions now. I guess we should just make #60019 have no effect on node16, even though node16 seems to have been incorrect all along. Node.js v16 is EOL now, so I think we should deprecate --module node16...

but I would add that I think that import attributes also should be allowed in node16, since otherwise users of >=node18 can only use the "unstable" nodenext.

It's this part that I'm worried about dropping in a patch version (even though, as a user, I do want it ASAP 🙂):

...and make a --module node18 that enforces use of import attributes for JSON imports (like nodenext does now).

@andrewbranch
Copy link
Member

No option will ever be dropped in a patch version. In fact, our current policy is that no option ever be dropped without a deprecation, and nothing ever be deprecated without a major version bump, which occurs once every 2.5 years.

@kirkwaiblinger
Copy link

kirkwaiblinger commented Dec 3, 2024

Great! So, would the thought be that a patch include

  1. a lenient "module: node16" that permits all of
    1. no import attributes/assertion at all
    2. import assertion
    3. import attributes
  2. unchanged "module: nodenext"
  3. potentially docs changes highlighting that this area is a meaningful discrepancy between node16 and nodenext

and the next minor include

  1. "module: node18" that requires JSON import attributes, and prohibits import assertions.
  2. No behavior changes to the "module: node16", which at this point will be in an extra-wrong state of permitting import attributes (extra-wrong due to the existence of node18 at this point). Or would import import attributes be re-banned at this point?

Sorry for pestering on this, but the expected resolution here informs some decision-making for me (on a node18 project that also needs to be upgraded from 'commonjs' to 'node16'/'nodenext') on whether to move it to

  • TS 5.6/node16
  • TS 5.7/nodenext, later TS 5.8/node18
  • (wait for) TS 5.7.3/node16
  • (wait longer for) TS 5.8/node18

@andrewbranch
Copy link
Member

andrewbranch commented Dec 3, 2024

I intend to patch node16 so it behaves the way it did in 5.6, while keeping nodenext the way it is in 5.7.2. Docs do need to be changed (I just overlooked the import assertions/attributes difference when writing those docs originally). Beyond that, the strategy needs to be discussed with the team.

@andrewbranch
Copy link
Member

Correction: I’d like to keep the “no named imports” part of #60019 in place for node16 in a patch, and only roll back the (unfulfillable) requirement of using import attributes/assertions.

@kirkwaiblinger
Copy link

Ok, this all makes sense. Thank you for the info!!

@silverwind
Copy link

silverwind commented Dec 8, 2024

It seems odd that nodenext and node16 are treated differently in 5.7 when the docs imply that they are aliases.

I would have preferred to specify node16 to future-proof the code in case nodenext is updated to alias a newer node version, but this change around JSON imports now forces me to use nodenext.

Edit: I saw #60673, which version does it apply to? 5.8?

@kirkwaiblinger
Copy link

@silverwind see also #60507.

@andrewbranch
Copy link
Member

@silverwind #60673 will be released in a 5.7 patch soon. You can always use typescript@next in the meantime. Also, see #60705 for 5.8

silverwind added a commit to go-gitea/gitea that referenced this issue Dec 10, 2024
Typescript 5.7 changed semantics around JSON imports and `nodenext` is
now [treated
differently](https://devblogs.microsoft.com/typescript/announcing-typescript-5-7-beta/#validated-json-imports-in---module-nodenext)
than `node16` for JSON imports and it requires the import attribute, so
change the value to that and add the attribute to eliminate this
typescript error.


[`moduleResolution`](https://www.typescriptlang.org/tsconfig/#moduleResolution)
is treated as an alias when `module` is `nodenext`, so we don't need to
specify it.

Also see microsoft/TypeScript#60589. It
appears the next Typescript release will fix this for `node16`, but I
guess it'll still be good to switch to `nodenext`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants