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

IsolatedDeclarations: emitted declarations inconsistent between transpileDeclaration API and TypeScript Playground #60031

Open
1 task done
Dunqing opened this issue Sep 23, 2024 · 1 comment
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@Dunqing
Copy link

Dunqing commented Sep 23, 2024

Acknowledgement

  • I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.

Example

NOTE: Enabled isolatedDeclaration and noCheck

Playground Link

// input
import { Comment } from '@vue/runtime-core'
//       ^^^^^^^ https://github.com/vuejs/core/blob/a177092754642af2f98c33a4feffe8f198c3c950/packages/runtime-core/src/vnode.ts#L70
export const comment: Comment 

// .d.ts output
export declare const comment: Comment;

This example is correct because the tsc checker analyzer Comment is a const variable defined in @vue/runtime-core. So the Comment who referenced by comment is actually referenced lib.dom.d.ts's Comment. But the output of this example in transpileDeclaration APi will same as the following example.

Another example

NOTE: Enabled isolatedDeclaration and noCheck

Playground Link

// input
import { Comment } from 'does-not-exist'
export const comment: Comment 

// .d.ts output
import { Comment } from 'does-not-exist';
export declare const comment: Comment;

This example is also correct because the tsc checker can't analyze what Comment is, so the Comment was kept in the output as it has been referenced by comment.

But If I want to use lib.dom.d.ts's Comment rather than imported it will be incorrect.

Expect Behaviour

In IsolatedDeclarations, Ideally, we shouldn't analyze imports, but if we stop analyzing, this will generate incorrect output.

So I suggest tsc should throw an error about the above cases when IsolatedDeclarations is enabled. This is also beneficial for third-party IsolatedDeclarations implementations.

BTW this issue I found when I fix the output mismatch between oxc-isolated-declarations and tsc

@Boshen
Copy link

Boshen commented Sep 23, 2024

This came from trying to align oxc's ID implementation in vue.

To reproduce:

  1. Run https://github.com/vuejs/core/blob/5e8898572fa63438b57a4b2b1de01a85dc49655e/package.json#L9
  2. The file https://github.com/vuejs/core/blob/main/packages/runtime-core/src/hydration.ts
import {
  Comment,
} from './vnode'

export enum DOMNodeTypes {
  ELEMENT = 1,
  TEXT = 3,
  COMMENT = 8,
}

export const isComment = (node: Node): node is Comment =>
  node.nodeType === DOMNodeTypes.COMMENT

Is IDed to

import { type VNode } from './vnode';
export declare enum DOMNodeTypes {
    ELEMENT = 1,
    TEXT = 3,
    COMMENT = 8
}
export declare const isComment: (node: Node) => node is Comment;

Very confusingly, the actual usage for the imported Comment is in https://github.com/vuejs/core/blob/a177092754642af2f98c33a4feffe8f198c3c950/packages/runtime-core/src/hydration.ts#L198. Comment in node is Comment is resolved to the one in lib.dom.d.ts

We are unable to implementation this behavior in oxc due to the lack of type checker.


Edit: I ended up patching vue: vuejs/core#12009

Boshen added a commit to Boshen/core that referenced this issue Sep 23, 2024
Related: microsoft/TypeScript#60031

oxc cannot replicate tsc's ID behavior because tsc accidently type
referenced the `Comment` to the global in
[dom.d.ts](https://github.com/microsoft/TypeScript/blob/88809467e8761e71483e2f4948ef411d8e447188/src/lib/dom.generated.d.ts#L5811-L5822)

This PR renames the imported `Comment` type to `VComment` for ID to behave correctly in a
non-type-reference context, as oxc will keep the imported `Comment` type.
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants