Skip to content

Improve portability of resolution-mode assertions automatically emitted in nodenext #55579

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

Closed
andrewbranch opened this issue Aug 30, 2023 · 4 comments · Fixed by #55725
Closed
Assignees
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@andrewbranch
Copy link
Member

In the last week, I’ve gotten two separate requests for help with solving an issue where a resolution-mode assertion in a triple-slash reference:

/// <reference types="chai" resolution-mode="require" />

was automatically inserted into a declaration file. The implementation file was an ES module that referenced a global defined in chai, which only ships as a CommonJS module. Presumably, the resolution-mode assertion gets added in declaration emit to increase the likelihood that, if the user installs a version of chai that contains separate ESM and CJS types, the ESM-format .d.ts file here will still resolve to the CJS types, as it did when it was created by the library’s compilation. (Note that if the user does have the same version of chai that the library had during compilation, the resolution-mode assertion will not actually be needed.)

The reason this was an issue for the two folks who reached out to me was that some of their users who consumed these declaration files were using --moduleResolution bundler or maybe even --moduleResolution node10, and consequently got the error:

error TS1452: 'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.

I feel pretty confident that non-node16/nodenext modes could simply ignore these assertions when they occur in declaration files. While we can’t guarantee that users in other modes will resolve these references the same way the library did, this is no different from any import statement the user wrote, or indeed any import() type automatically inserted by the declaration emitter.

Maybe better, we could also consider supporting resolution-mode assertions and the way they affect conditional package.json "exports" lookups in every moduleResolution mode in type reference directive lookups. They don’t reflect a runtime module resolution feature, so it feels like all we should care about is doing our best to resolve to the same global types in every moduleResolution mode.

It should be noted that in the meantime, a workaround for library authors is to manually write a /// <reference types="chai" /> with no resolution-mode assertion in the implementation file that generated the reference that caused the problem.

@andrewbranch andrewbranch added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Aug 30, 2023
@andrewbranch andrewbranch self-assigned this Aug 30, 2023
@andrewbranch
Copy link
Member Author

It should be noted that in the meantime, a workaround for library authors is to manually write a /// <reference types="chai" /> with no resolution-mode assertion in the implementation file that generated the reference that caused the problem.

I just learned that this doesn’t always work, for some reason. I haven’t looked into what the difference is, but

// @Filename: index.mts

/// <reference types="node" />
export function f(s: NodeJS.ReadableStream) {}

emits a declaration file with

/// <reference types="node" resolution-mode="require"/>
export function f(s: NodeJS.ReadableStream) {}

@mpodwysocki
Copy link
Member

@andrewbranch Yes, this bug is holding up our transition to ESM for the Azure SDK for JS Azure/azure-sdk-for-js#26238 as we cannot get our sdk/test-utils/perf/utils/utils.ts to compile in such a way that both ESM and CJS can use it.

By default, the utils.ts is the following:

/// <reference types="node" />

export async function drainStream(stream: NodeJS.ReadableStream): Promise<void> {

When compiled it will emit the following into the utils.d.ts:

/// <reference types="node" resolution-mode="require"/>
/// <reference types="node" resolution-mode="require"/>

For some reason, unlike the chai fix we have, this does not work with the Node types. We need this fixed so we can upgrade our core from CJS to ESM.

@jetwiwo
Copy link

jetwiwo commented Sep 11, 2023

Ran into this trying to make a library for use in cloudflare workers. It uses "types": [ "@cloudflare/workers-types" ] to make the correct ambient types available, but because I'm using nodenext module resolution my library becomes incompatible with bundler resolution downstream. Is there a particular reason that bundler mode can't either support or ignore resolution-mode assertions at least?

@andrewbranch
Copy link
Member Author

We tentatively decided that all modes should support resolution-mode assertions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
3 participants