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

Incorrect .js suffix added to named module import in declaration files. #60930

Closed
MicahZoltu opened this issue Jan 7, 2025 · 3 comments
Closed
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Jan 7, 2025

🔎 Search Terms

declaration js extension named module node10

🕗 Version & Regression Information

  • I was unable to test this on prior versions because I'm lazy and I suspect this bug has existed since module type node10 was introduced since it is somewhat obscure.

⏯ Playground Link

Not possible because repro requires multiple files.

💻 Code

//index.ts
import { secp256r1 } from '@noble/curves/p256'
import { apple } from './other.js'
export const noble = secp256r1
apple
// other.ts
export const apple: number = 5
{
    "compilerOptions": {
        "module": "ESNext",
        "target": "ES2024",
        "moduleResolution": "Node10",
        "emitDeclarationOnly": true,
        "declaration": true,
    }
}

🙁 Actual behavior

The generated .d.ts file has

export declare const noble: import("@noble/curves/_shortw_utils.js").CurveFnWithCreate;

but this is invalid because there is no named export of @noble/curves/_shortw_utils.js. In particular, there .js suffix should not be added.

🙂 Expected behavior

The generated .d.ts file doesn't include an extraneous .js on the dynamic import statement.

export declare const noble: import("@noble/curves/_shortw_utils").CurveFnWithCreate;

Additional information about the issue

The reason I think this is a bug and not just some esoteric behavior of node10 module resolution is that if you do not import other.js, it correctly emits the following line instead (note the missing .js extension):

export declare const noble: import("@noble/curves/_shortw_utils").CurveFnWithCreate;

Either it is correct with the .js extension (a Node10 module resolution thing), or it is correct without the .js extension, but it doesn't make sense that the presence of an extension would changed based on whether or not a local file was imported.

Note: Bug does not reproduce if you import from another package. It only seems to occur if you import from a relative path (which is why there is no playground repro).

@RyanCavanaugh RyanCavanaugh added the Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it label Jan 7, 2025
@RyanCavanaugh
Copy link
Member

We're very likely to deprecate node10 in 6.0. I don't think this warrants a fix at this point.

@MicahZoltu
Copy link
Contributor Author

That seems reasonable to me (deprecating in 6.0, not fixing bug prior to that), thanks for sharing the plan!

Am I correct in assuming that the .js suffix is a bug, and not something intended for node10?

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Won't Fix" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2025
MicahZoltu added a commit to MicahZoltu/viem that referenced this issue Jan 14, 2025
Currently, the `_types` is built using `node` module resolution, which results in this project generating invalid types.  Details about the TypeScript bug can be found at microsoft/TypeScript#60930 (comment) and there are no plans to fix it since `node` (which is equivalent to `node10`) is being deprecated in TypeScript soon.

The rest of this project uses `nodenext`, so this changes the types `tsconfig` file to also use `nodenext`, and fix the bug that currently prevents the library from being used correctly in native ESM projects.
MicahZoltu added a commit to MicahZoltu/ox that referenced this issue Jan 15, 2025
Currently, the `_types` is built using `node` module resolution, which results in this project generating invalid types.  Details about the TypeScript bug can be found at microsoft/TypeScript#60930 (comment) and there are no plans to fix it since `node` (which is equivalent to `node10`) is being deprecated in TypeScript soon.

The way this bug manifests is in files like `src/_types/core/P256.d.ts` where it would previously generate code like:�```ts
create: (hash: import("@noble/curves/abstract/utils.js").CHash) => import("@noble/curves/abstract/weierstrass.js").CurveFn;
```
when it should have generated code like this (which is what is generated after this change):�```ts
create: (hash: import("@noble/curves/abstract/utils").CHash) => import("@noble/curves/abstract/weierstrass").CurveFn;
```

For the cjs build command, I changed from `node` to `node10` to be more explicit/clear, since `node` was just an alias for `node10`.

For the esm build command, I changed it to use the tsconfig so it is less likely to get out of sync with the generated types.

Also updated .gitignore to ignore `.pnpm-store` which had 30,000 files it wanted to commit after I did `pnpm install` using the recommended version of `pnpm`.
jxom pushed a commit to wevm/ox that referenced this issue Jan 15, 2025
Currently, the `_types` is built using `node` module resolution, which results in this project generating invalid types.  Details about the TypeScript bug can be found at microsoft/TypeScript#60930 (comment) and there are no plans to fix it since `node` (which is equivalent to `node10`) is being deprecated in TypeScript soon.

The way this bug manifests is in files like `src/_types/core/P256.d.ts` where it would previously generate code like:�```ts
create: (hash: import("@noble/curves/abstract/utils.js").CHash) => import("@noble/curves/abstract/weierstrass.js").CurveFn;
```
when it should have generated code like this (which is what is generated after this change):�```ts
create: (hash: import("@noble/curves/abstract/utils").CHash) => import("@noble/curves/abstract/weierstrass").CurveFn;
```

For the cjs build command, I changed from `node` to `node10` to be more explicit/clear, since `node` was just an alias for `node10`.

For the esm build command, I changed it to use the tsconfig so it is less likely to get out of sync with the generated types.

Also updated .gitignore to ignore `.pnpm-store` which had 30,000 files it wanted to commit after I did `pnpm install` using the recommended version of `pnpm`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

No branches or pull requests

3 participants