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

fix(types): fix import('echarts/types/dist/shared') in users' .d.ts can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0 #20030

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

100pah
Copy link
Member

@100pah 100pah commented Jun 13, 2024

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fix that In users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0.

Fixed issues

Fix #19663 .
Relative issues: ecomfe/vue-echarts#766 , #19513

Details

Before: What was the problem?

In users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0.
It cause that users can only get type any when import echarts type.

After: How does it behave after the fixing?

After some investigate to tsc, I found that:

Test case (tested in typescript v5.3.3 and v4.6.4) :

  • Case_A:
    /** @file main1.ts (user's source ts files) */
    import { init } from "echarts/core";
    type EChartsType1 = ReturnType<typeof init>;
    export let option = {} as EChartsType1;
    /** @file main1.d.ts (tsc generated d.ts file) */
    // Has inline import() generated, and refer deeply to "echarts/types/dist/shared",
    // in which `EChartsType` is declared literally.
    export declare let option: import("echarts/types/dist/shared").EChartsType;
  • Case_B:
    /** @file main2.ts (user's source ts files) */
    import { init } from "echarts/core";
    type InitType = typeof init;
    type EChartsType1 = ReturnType<InitType>;
    export let option: EChartsType1; // This is the only difference from "main1.ts".
    /** @file main2.d.ts (tsc generated d.ts file) */
    // No inline import() generated.
    import { init } from "echarts/core";
    type EChartsType1 = ReturnType<typeof init>;
    export declare let option: EChartsType1;
    export {};

In the case about we can found that:

  • tsc may generate "inline import" (i.e., import(xxx/xxx/xxx)) or not.
  • When generate inline import import(xxx/xxx/xxx), the path xxx/xxx/xxx probably goes deep into the file where the target type declared literally defined, but may not consider its visibility defined in "exports" field of package.json.
    • Take the Case_A above as am example, the target type need to be resolved is EChartsType from the last sentence export let option = {} as EChartsType1;, and EChartsType is declared in echarts/types/dist/shared.d.ts, the final inline import is import('echarts/types/dist/shared') or import('echarts/types/dist/shared.js'), although echarts/types/dist/shared seems to be a private file and imported by echarts/core.d.ts.
    • There is some investigation into the source code of tsc. It's verbose, just for the record:
      • When travel and parse ts file starting from transformRoot,
      • in which function symbolToTypeNode is called to resolve the literal type EChartsType1 of the sentence export let option = {} as EChartsType1;,
      • in which lookupSymbolChain is called to use the input symbol(of "EChartsType") to get a chain like [symbol(of "dist/shared.d.ts"), symbol(of "EChartsType")], and generate a specifier for chain[0], and call const lit = factory.createLiteralTypeNode(factory.createStringLiteral(specifier)); to create a literal node containing the path 'xxx/dist/shared', and call return factory.createImportTypeNode(lit, ...) to return a new type node on which the literal node is mounted.
      • So for the original type node "EChartsType" has been resolved the a type node "xxx/dist/shared".
      • Finally in writeKeyword("import"); writePunctuation("("); emit(node.argument); the literal node of 'xxx/dist/shared' generated above is in the node.argument and be used to write as import('xxx/dist/shared').
    • Both ambient module declaration file or normal module file has the same behavior.
      /** @file echarts/core.d.ts (An ambient module declarataion file) */
      declare module "echarts/core" {
          export * from 'echarts/types/dist/core';
      }
      /** @file echarts/core.d.ts (An normal module file) */
      export * from './types/dist/core';
  • Either import('echarts/types/dist/shared') or import('echarts/types/dist/shared.js') can be generated by tsc.
    • For the case import('echarts/types/dist/shared.js'):
      • Generated like that when setting "moduleResolution": "node" in "tsconfig.json".
      • Although this file does not exist, it seems OK in d.ts file to used it, tsc can resolve it back to echarts/types/dist/shared.d.ts (tested in tsc v4.6.4 & v5.3.3).
    • For the case import('echarts/types/dist/shared'):

Other infomation

In https://cdn.jsdelivr.net/npm/vue-echarts@6.7.3/dist/index.d.ts there are both import("echarts/types/dist/shared") and import("echarts/types/dist/echarts"). It because in https://github.com/ecomfe/vue-echarts/blob/v6.7.3/src/types.ts#L2 both from 'echarts/core' and 'echarts'.

import { init } from "echarts/core";
import type { SetOptionOpts, ECElementEvent, ElementEvent } from "echarts";

It's OK, but it probably be better a little if all of them are from 'echarts/core', @Justineo .

In the user's vue project, after update the dependent echarts, if the type is still any in .vue file in vscode, try to "> Vue: Restart Vue and TS servers"

Copy link

echarts-bot bot commented Jun 13, 2024

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

The pull request is marked to be PR: author is committer because you are a committer of this project.

To reviewers: If this PR is going to be described in the changelog in the future release, please make sure this PR has one of the following labels: PR: doc ready, PR: awaiting doc, PR: doc unchanged

This message is shown because the PR description doesn't contain the document related template.

@100pah 100pah changed the title Fix that In users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0. Fix that in users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0. Jun 13, 2024
@100pah 100pah changed the title Fix that in users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0. Fix that in users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0 Jun 13, 2024
Copy link
Contributor

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-20030@a209486

@Ovilia Ovilia merged commit b61f6c0 into master Jun 14, 2024
2 checks passed
Copy link

echarts-bot bot commented Jun 14, 2024

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

@Ovilia Ovilia deleted the fix/ts-type-visibility branch June 14, 2024 05:41
@plainheart plainheart added this to the 5.5.1 milestone Jun 18, 2024
@plainheart plainheart changed the title Fix that in users' .d.ts import('echarts/types/dist/shared') can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0 fix(types): fix import('echarts/types/dist/shared') in users' .d.ts can not visit 'echarts/types/dist/shared.d.ts' since v5.5.0 Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Duplicated type declarations in dist type files
3 participants