TS2834: Relative import paths need explicit file extensions in EcmaScript imports...

[tpluscode]

... when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

I have been fixing multiple packages which show this error in an ESM project with module resolution = node16

For example: #65918

I think this should be enforced in type packages which are type: module in their package.json

Re #module

[tpluscode]

This has again bit me, although slightly different. I updated the package @types/rdfjs__serializer-jsonld-ext to its new, ESM version. It imports jsonld/jsonld-spec. Not a relative path but one that fails to resolve nonetheless unless it's changed to jsonld/jsonld-spec.js

Thoughts? @peterblazejewicz @sheetalkamat @jakebailey @sandersn

[peterblazejewicz]

tbh I'm using only client (no Node host code, except of a bunch of bundler specific files) nowadays, but I know topic is complex and ships with dedicated detailed page:
https://www.typescriptlang.org/docs/handbook/modules/reference.html#module-format-detection
the behaviour you're seeing could reflect current implementation. Let's wait for MS folks thoughts

[tpluscode]

Ah, thanks for the link. It made me check if jsonld exports that module. Turns out it is not ESM and it does not actually have a module ./jsonld-spec.js at all. This exists only in the types package. Why does it require .js extension? 🤔

[jakebailey]

I am not clear what you're describing; is the fix not to just switch the module mode to node16?

[tpluscode]

You refer to the jsonld problem now? This is the project: https://github.com/zazuko/rdf-utils-fs

It is the opposite. When module is set to Node16 or NodeNext, the build fails on the import of "jsonld/jsonld-spec"

[jakebailey]

I was referring to the example in the original issue; happy to look at anything else if they pose a better example.

Though, that is a valid error, no? What isn't enforced? (We eventually want every package on DT to use node16 resolution)

[tpluscode]

What isn't enforced?

I think it's the same rule as with packages using modules. Relative imports must include the extension. I think it should be enforced by default by DT.

[jakebailey]

If a package doesn't have type=module, then its .ts files are considered to be CJS and don't need the extensions; importers should see that as well and correctly resolve the import. So, I'm not entirely certain what behavior you're seeing that would violate that rule. In module mode, yes, definitely, you need the extension, but that only happens when type=module or .mts, no?

I think this should be enforced in type packages which are type: module in their package.json

And this should happen if the tsconfig on DT is changed to have module=node16, which is what we'd like to switch everything to.

[tpluscode]

I think we're talking past each other. The original example was @types/rdf-ext which is type: module, matching the rdf-ext package:

DefinitelyTyped/types/rdf-ext/package.json

Line 8 in 77fde50

"type": "module",

Form what I can tell, it doesn't matter what we have in tsconfig.json in the DT package. It comes down to the downstream package which imports these types. If I installed @types/rdf-ext prior to #65918, it would cause compilation errors when I set module=Node16 or NodeNext.

Thus, I it seems sensible to me to enforce .js on all DT packages which represent modules. It will prevent errors when using module=node(16|next) and shouldn't affect anyone else. I think 🤞

[jakebailey]

And what I'm trying to say is that because this package has set module=commonjs (

DefinitelyTyped/types/rdf-ext/tsconfig.json

Line 3 in 77fde50

"module": "commonjs",

), this was not caught because the extension was not required. If you flip that to node16, you get errors when doing pnpm test rdf-ext, including:

Cannot find module 'rdf-ext/lib/Literal' or its corresponding type declarations             @definitelytyped/expect
   24:26  error  [email protected], 4.8, 4.9, 5.0, 5.1, 5.2, 5.3, 5.4 compile error: 
Cannot find module 'rdf-ext/lib/NamedNode' or its corresponding type declarations           @definitelytyped/expect
   25:21  error  [email protected], 4.8, 4.9, 5.0, 5.1, 5.2, 5.3, 5.4 compile error: 
Cannot find module 'rdf-ext/lib/Quad' or its corresponding type declarations                @definitelytyped/expect
   26:25  error  [email protected], 4.8, 4.9, 5.0, 5.1, 5.2, 5.3, 5.4 compile error: 
Cannot find module 'rdf-ext/lib/Variable' or its corresponding type declarations            @definitelytyped/expect
  264:5   error  [email protected], 4.8, 4.9, 5.0, 5.1, 5.2, 5.3, 5.4 expected type to be:

Which are resolved when adding a .js extension to the imports.

If a DT package checks in node16 mode, then other projects which import it and also set node16 must also necessarily compile.

[tpluscode]

Ooooh, how did I never realise that it does actually make a difference for pnpm test what we have in the tsconfig 🤡

Ok, in such case I shall reformulate my proposal. I think it should be enforced that if the package.json has type=module, then module in tsconfig must not be commonjs. I say in that way because I think it should also be fine to have ES2019+ ro something, and that too should trigger the import check for extensions

[tpluscode]

Well ok, I see that pnpm test only allows cjs or node 16

When "module" is present, it must be set to "commonjs" or "node16"

Why is that, actually? Convenience?

[jakebailey]

To be clear, we want every package to eventually have module=node16, regardless of the type in package.json as that will have maximal compatibility for downstream users. There's just 9000+ packages so we haven't done it and the ability to do so is very new (it's what the switch to a pnpm monorepo unlocked).

When "module" is present, it must be set to "commonjs" or "node16"

Why is that, actually? Convenience?

commonjs was previously the only valid option, and we added node16. nodenext is a bad idea because it's unpinned (though, is still equivalent to node16 until node changes their module system again). The other options are not good for libraries.

[tpluscode]

Gotcha

To be clear, we want every package to eventually have module=node16

Depending on the timeline, maybe it would be possible to only enforce node16 on pull requests. Gradually, at least actively maintained packages would naturally migrate to the desired state

[jakebailey]

I think more likely would be for us to enable module=node16 on every package that passes, and update dts-gen to set node16, leaving only broken packages that need to be fixed.

We're also going to be adding AreTheTypesWrong to DT CI, which will help.

[peterblazejewicz]

@tpluscode Tom, you happy with the outcome? Convert to discussion?

[tpluscode]

Yes, you can convert to discussion. Thanks