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

[api-extractor] ts.getResolvedModule with TypeScript 5.3 #4404

Closed
JoshuaKGoldberg opened this issue Oct 23, 2023 · 11 comments · Fixed by #4408
Closed

[api-extractor] ts.getResolvedModule with TypeScript 5.3 #4404

JoshuaKGoldberg opened this issue Oct 23, 2023 · 11 comments · Fixed by #4408
Labels
enhancement The issue is asking for a new feature or design change

Comments

@JoshuaKGoldberg
Copy link

JoshuaKGoldberg commented Oct 23, 2023

Summary

I'm trying to run api-extractor on a repository with TypeScript 5.3 as a dev dependency. It was previously working with TypeScript 5.2.

Repro steps

See typescript-eslint/typescript-eslint#7821:

  1. Check out that PR branch & commit
  2. yarn
  3. yarn build (specifically, the packages/ast-spec task)

Expected result: Successful API extraction / build?

Actual result:

packages/ast-spec $ yarn build

api-extractor 7.38.0  - https://api-extractor.com/

Using configuration from ./api-extractor.json
Analysis will use the bundled TypeScript version 5.3.0-beta

ERROR: ts.getResolvedModule is not a function

packages/ast-spec $

Details

I'm guessing this is because of the (ts as any).getResolvedModule used to access TypeScript compiler internals:

// Compiler internal:
// https://github.com/microsoft/TypeScript/blob/v4.7.2/src/compiler/utilities.ts#L161
return (ts as any).getResolvedModule(sourceFile, moduleNameText, mode);

microsoft/TypeScript#55790 removed that internal API.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/api-extractor version? Both 3.6.1 and 3.8.0
Operating system? Mac
API Extractor scenario? rollups (.d.ts)
Would you consider contributing a PR? Yes
TypeScript compiler version? 5.3.0-beta
Node.js version (node -v)? 20 (n/a)
@octogonz
Copy link
Collaborator

octogonz commented Oct 23, 2023

Just to clarify, did you encounter this error while making a PR for API Extractor to upgrade its bundled version? Looking at the code, the bundled version is currently supposed to be 5.0.x. (We do need to update that.)

(In other words, are you reporting a bug that API Extractor loaded the wrong TypeScript version, instead of the bundled version? Or are you asking for help making an API Extractor PR to support the latest TypeScript?)

@JoshuaKGoldberg
Copy link
Author

Or are you asking for help making an API Extractor PR to support the latest TypeScript?

This one: that it's crashing on the latest beta version of TypeScript.

@octogonz
Copy link
Collaborator

Ok, I can take a look at it tomorrow morning. Most likely we just need to find what happened to that internal API in the latest branch.

@octogonz
Copy link
Collaborator

Right, the getResolvedModule() API was removed from utilities.ts in microsoft/TypeScript#55790 (which is not been officially released yet, and only affects the daily/beta releases to date).

It seems that @sheetalkamat later exposed an equivalent API as an /** @internal */ member of ts.program in microsoft/TypeScript#55818. That means we now need a ts.program object in order to call the API, but this is conveniently already available in the two places in ExportAnalyzer that need it.

API Extractor's tests seem to pass with this small change. I'll make a PR for you to try.

We don't usually do official releases of API Extractor bundling beta versions of the TypeScript compiler. And (for exactly the reasons seen here) we don't provide a way for API Extractor users to override the compiler version.

@JoshuaKGoldberg are you just doing investigative testing? Or are you planning to publish a release of your project? If so, maybe we need to publish a "beta" release of API Extractor that you can depend on? Let me know.

@octogonz
Copy link
Collaborator

Try this: #4408

@octogonz octogonz added the enhancement The issue is asking for a new feature or design change label Oct 24, 2023
@JoshuaKGoldberg
Copy link
Author

planning to publish a release of your project

Yup, this one. We publish a new version of typescript-eslint every Monday, and aim to have support for each new version of TypeScript as soon as possible (https://typescript-eslint.io/users/dependency-versions#typescript).

Try this: #4408

After manually applying the changes in local node_modules/:

    ✖  nx run ast-spec:build
       api-extractor 7.38.0  - https://api-extractor.com/
       
       Using configuration from ./api-extractor.json
       Analysis will use the bundled TypeScript version 5.3.0-beta
       
       ERROR: Internal Error: getResolvedModule() could not resolve module name "./base/Accessibility"
       /Users/josh/repos/typescript-eslint/packages/ast-spec/dist/index.d.ts:1:1
       You have encountered a software defect. Please consider reporting the issue to the maintainers of this application.
       Warning: run-commands command "yarn build" exited with non-zero status code

@octogonz
Copy link
Collaborator

Yup, this one. We publish a new version of typescript-eslint every Monday, and aim to have support for each new version of TypeScript as soon as possible (https://typescript-eslint.io/users/dependency-versions#typescript).

@JoshuaKGoldberg okay but in general, API Extractor's bundled version of typescript does NOT need to be >= the version in use by your project. API Extractor reads the .d.ts files of your project, the way an external consumer would, and of course you don't mandate that consumers of typescript-eslint must have the absolute latest version of typescript. The TypeScript compiler normalizes .d.ts files during emit, and also they have a simplified syntax compared to regular .ts files. And also, API Extractor is designed so that its analysis (including .d.ts rollups) will generally work correctly even the file contains unrecognized syntaxes.

@octogonz
Copy link
Collaborator

octogonz commented Oct 25, 2023

In fact, looking at your branch, I think the whole problem is that your yarn resolutions are producing an incorrect installation of API Extractor:

API Extractor's package.json file requests:

  "dependencies": {
    "@microsoft/tsdoc": "0.14.2",
    "@microsoft/tsdoc-config": "~0.16.1",
    "colors": "~1.2.1",
    "lodash": "~4.17.15",
    "resolve": "~1.22.1",
    "semver": "~7.5.4",
    "source-map": "~0.6.1",
    "typescript": "~5.0.4",  // 👈👈👈
    "@microsoft/api-extractor-model": "7.28.2",
    "@rushstack/node-core-library": "3.61.0",
    "@rushstack/rig-package": "0.5.1",
    "@rushstack/ts-command-line": "4.16.1"
  },

But your resolution is like:
image

I tried doing a normal npm install @microsoft/api-extractor in another folder and then symlinking your API Extractor installation to use that typescript version:
image

And then your project builds just fine using TypeScript 5.0.4:
image

Conclusion

  • We should improve API Extractor to work with the latest TypeScript 👍
  • HOWEVER, your project has a wrong installation that does not respect the SemVer ranges requested by package.json files. This is most likely caused by the "resolutions" field from the package.json file in your project root folder. In trying to override some versions, those overrides are overreaching and breaking the functionality of dependencies. If you fix that problem, you can use the current API Extractor without any changes.

@JoshuaKGoldberg
Copy link
Author

Got it, thanks! I really appreciate you debugging this for us ❤️. Filed an issue on our end to not take up too much space on the rushstack issue tracker. typescript-eslint/typescript-eslint#7839

@octogonz
Copy link
Collaborator

octogonz commented Oct 26, 2023

After manually applying the changes in local node_modules/:

@JoshuaKGoldberg with the PR build of API Extractor, I am not able to reproduce the error that you reported.

C:\GitEx\typescript-eslint> nx run ast-spec:build

> nx run ast-spec:build

api-extractor 7.38.0  - https://api-extractor.com/

Using configuration from ./api-extractor.json
Analysis will use the bundled TypeScript version 5.3.0-beta
API Extractor completed successfully

 ———————————————————————————————

 >  NX   Successfully ran target build for project ast-spec (7s)


Could you provide more detailed repro instructions? I used typescript-eslint/typescript-eslint@e198efb

@JoshuaKGoldberg
Copy link
Author

Hmm, at this point it's likely I misunderstood something and/or the fix you suggested around resolutions would fix things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants