[api-extractor] Support .d.mts for documentation and rollups

mscharley commented 2 years ago

Summary

I've been messing around with compiling typescript to .mjs and ran into some issues with the rollups/api extraction features:

$ npx api-extractor run --local --diagnostics
api-extractor 7.19.2  - https://api-extractor.com/

Using configuration from ./api-extractor.json

ERROR: Error parsing ./packages/foo/api-extractor.json:
The "mainEntryPointFilePath" value is not a declaration file: ./packages/foo/dist/index.d.mts

Repro steps

Create a simple typescript project.
Instead of using .ts files, name them as .mts.
When compiled, these will result in .mjs, .d.mts and appropriate map files.
Attempt to run api-extractor on the results.

Expected result: API extractor will work as normal.

Actual result: The filename fails validation - I'm not sure if this runs deeper, but the actual contents hasn't seemed to change, other than imports/exports will all refer to ./foo.mjs instead of ./foo.

Details

Standard questions

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

Question	Answer
`@microsoft/api-extractor` version?	7.19.2
Operating system?	Arch Linux
API Extractor scenario?	reporting and rollups
Would you consider contributing a PR?	Potentially, if just the filename validation needs updating
TypeScript compiler version?	4.5.4
Node.js version (`node -v`)?	16.13.1

mscharley commented 2 years ago

A quick hack inside node_modules to disable this validation check and instead I get the following error instead so I suspect this is more than just a simple regex update:

ERROR: Unable to load file: ./dist/index.d.mts

$ cat ./dist/index.d.mts
import { BaseColour } from './Colour.mjs';
export { BaseColour, BaseColour as BaseColor };
import type { Colour } from './Colour.mjs';
export type { Colour, Colour as Color };
import type { Colours } from './Colours.mjs';
export type { Colours, Colours as Colors };
import type { ColourSpace } from './ColourSpace.mjs';
export type { ColourSpace, ColourSpace as ColorSpace };
export * from './constructors.mjs';
export { HSB } from './HSB.mjs';
export { HSL } from './HSL.mjs';
export { RGB } from './RGB.mjs';
export { interpolator } from './interpolator.mjs';
//# sourceMappingURL=index.d.mts.map

mscharley commented 2 years ago

And, digging further I've just realised that this is all predicated on being on typescript@next which I'm not yet and probably puts this well out of scope for tooling for a while so now I'm not sure what to do with this ticket.

michaelfaith commented 1 year ago

I'm facing this as well. If the type declaration file is .d.cts or .d.mts API Extractor throws this error

The "mainEntryPointFilePath" value is not a declaration file: <file path>

michaelfaith commented 1 year ago

And, digging further I've just realised that this is all predicated on being on typescript@next which I'm not yet and probably puts this well out of scope for tooling for a while so now I'm not sure what to do with this ticket.

@mscharley can you explain what you mean by this? I'm using TS 5.0.4 and seeing this for both cts and mts. I believe these extensions were introduced in 4.7(?), so it's interesting that this hasn't been supported here yet.

octogonz commented 1 year ago

Implementing this support seems fairly straightforward. However I haven't personally encountered any projects using .cts or .mts file extensions in any of the corporate repos that I work with. Just curious, why aren't you able to use the .ts file extension?

If someone wants to contribute a PR, we would accept it.

michaelfaith commented 1 year ago

@octogonz we're using this on large reusable component library projects, that are being dual-bundled with both cjs and esm versions of each entry point. For the esm entry points we've standardized around mjs for the js files and .d.mts for the type declarations (since that's what typescript creates since v4.7), and for the commonjs entry points cjs and .d.cts. Technically we could change the extensions to .d.ts since they're in separate folders in the package, and we're using export conditions, but we like the explicit nature of having the extensions communicate the module type.

If nobody else takes it up by the weekend, I'll take a look. I wouldn't mind helping with this.

michaelfaith commented 1 year ago

I had some time to work on it last night.

mscharley commented 1 year ago

And, digging further I've just realised that this is all predicated on being on typescript@next which I'm not yet and probably puts this well out of scope for tooling for a while so now I'm not sure what to do with this ticket.

@mscharley can you explain what you mean by this? I'm using TS 5.0.4 and seeing this for both cts and mts. I believe these extensions were introduced in 4.7(?), so it's interesting that this hasn't been supported here yet.

@michaelfaith I'm not sure if something changed or my understanding has just gotten better in the last 18 months since I originally posted this, but you're correct that today nothing special should be needed. The only things I'm aware of that are needed to get the cjs/mjs extensions working correctly and consistently in TypeScript is to set the correct type in package.json in order to set the default module type and "moduleResolution": "node16" or later in tsconfig.json.

And seconding the reply to @octogonz : the main reason to need these extensions for API Extractor is for dual releases of commonjs and ESM modules in the same package using eg. the exports key in package.json. I've run into some edge cases where it's useful to have different exports and hence types/documentation for each export style in the past, though none of the current libraries I'm working on need it any more. #3557 would be really useful for that use-case as well.

michaelfaith commented 1 year ago

3557 would be really useful for that use-case as well.

Agreed. This would be great also. We've gotten around this limitation by running api-extractor from a node script that builds out an array of entry points and iteratively calls prepare and invoke for each entry point. But it'd be nice to have that capability built-in.

michaelfaith commented 1 year ago

For anyone using the API (instead of the cli), you can monkey-patch the following function to get this working, before the PR is merged / released:

ExtractorConfig.hasDtsFileExtension = (filePath: string): boolean => {
    const dtsRegex = /\.d\.[mc]?ts$/i;
    return dtsRegex.test(filePath);
};

ryanbas21 commented 1 year ago

I ran into this issue when trying to configure vite to bundle my types. the dts plugin uses this under the hood if you choose to bundle the types.

For packages that publish cjs and mjs versions, this is required as you need to provide both a mjs version and cjs version (or depending on how you set type in package.json a d.ts and d.(m|c)ts version.

If this is a small change I'd be happy to PR it just gotta figure out where to look

michaelfaith commented 1 year ago

@ryanbas21 feel free to review the PR I already put in (https://github.com/microsoft/rushstack/pull/4196); I believe it should meet your needs.

microsoft / rushstack