-
Notifications
You must be signed in to change notification settings - Fork 608
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] Generate declaration files in-memory as part of extraction #1010
Comments
This is an interesting idea. Some thoughts:
Otherwise, this proposal seems useful and like a good idea. |
Your answer makes me thing that this feature does not really belong to api-extractor. Rather, we should expose a programmatic API so that one can write extensions to third-party tools that oversee compilations. For example a webpack plugin for ts-loader would directly hook itself to the declaration files generated in memory and feed to api-extractor. I think this is the better approach as that would mean less code to maintain and would solve every single issue you addressed:
For that to work api-extractor must abstract away all filesystem access. It should also behave nicely when a single What do you think? |
Sure, I like this idea a lot. 👍 |
Since tsc can already run in browser and emit files into memery, I think if we can make api-extractor resolve/load files from memory(depend on a in-memory filesystem), both this issue and #1828 would be resolved. |
The way
For the TSDoc project, all the config file parsing was moved into None of this is particularly difficult work, but it would touch a lot of files. If someone wants to work on this we would certainly support it and provide help. |
I would love to help. Current It definitely need a lot of file-moving and refactoring. What do you suggest to start with? I guess the entry point is these lines. If I get these 10 lines works, I can get the apiModel object in browser, which is the core demand for in-browser usage. The apiReport and dtsRollup feature will not be brought into |
@csr632 Before trying to get it exactly right, it might be easier to start with a prototype. Like make a branch that tries to carve out the core API Extractor engine, moving all the non-portable stuff behind an interface. Once you get it running, we can step back and look at what's in the interface, and what uses it, and where the boundary should be. Then we can come back and make a PR that separates the code for real. This page explains how to invoke the API Extractor engine from a script. And this script is a good example to study. If you can get those interfaces running in a web browser, you have pretty much the whole functionality of API Extractor. |
Also, it would be good to confirm that the |
I have ported This is warning from rollup:
These circular references are causing The circular references lead to this line in bundle, which is an "access before initialization". Probably because I changed this line into this line. But I don't want to solve circular dependency by using commonjs |
@csr632 I didn't have time to do a full analysis. But starting with ApiItem.ts, I see 3 imports that are clearly pulling in things that depend circularly back on import { ApiPackage } from '../model/ApiPackage';
import { ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
import { ApiItemContainerMixin } from '../mixins/ApiItemContainerMixin';
public getAssociatedPackage(): ApiPackage | undefined {
for (let current: ApiItem | undefined = this; current !== undefined; current = current.parent) {
if (current.kind === ApiItemKind.Package) {
return current as ApiPackage;
}
}
return undefined;
} In TypeScript 3.8 we could make this explicit using For both if (ApiParameterListMixin.isBaseClassOf(current)) {
reversedParts.push('()');
} and public getMergedSiblings(): ReadonlyArray<ApiItem> {
const parent: ApiItem | undefined = this._parent;
if (parent && ApiItemContainerMixin.isBaseClassOf(parent)) {
return parent._getMergedSiblingsForMember(this);
}
return [];
} So I think you could solve those by moving the
public static deserialize(jsonObject: IApiItemJson, context: DeserializerContext): ApiItem {
// The Deserializer class is coupled with a ton of other classes, so we delay loading it
// to avoid ES5 circular imports.
// eslint-disable-next-line @typescript-eslint/no-var-requires
const deserializerModule: typeof import('../model/Deserializer') = require('../model/Deserializer');
return deserializerModule.Deserializer.deserialize(context, jsonObject);
} Hmm.... this problem is harder to solve. Webpack does support delayed imports, but they return promises. They are not synchronous like CommonJS Maybe we could define empty stubs in ApiItem.ts and fix them up later. |
Take a look at this prototype: I think a solution like this works reasonably well, and the code is not too crazy for people to understand. |
@octogonz We could create can This pattern can solve the problem because we can control the module eval order in // import order matters!
export * from './items/ApiItem.ts'
export * from './model/DeserializerContext'
// re-export other relavant modules...... If some module depend on ApiItem, import it from This way we can avoid large refactor. We only need to modify some import statement. |
microsoft/rushstack#1010 (comment) This commit only changes some import statements in source code.
https://github.com/csr632/api-extractor-core |
Has anyone made any progress on this? From my research: You can write the DTS files in-memory by following this: https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API#getting-the-dts-from-a-javascript-file. (note: change the Then all the file operations for A proper solution would involve exposing the instance via dep injection, but would require a lot of plumbing work I suspect. The proper proper solution would be to make a layer in front of the For now, I am going to try monkey-patch it, and if that fails, then I will use something like |
This is a feature request.
Proposed change
Currently api-extractor requires that we use the typescript compiler to generate our declarations first. Then, api-extractor reads those declarations and does its magic.
My proposal is instead to run
tsc
withemitDeclarationOnly
as a preliminary step. The files emitted would be written to an in-memory filesystem plugged by api-extractor. Those files would then be read from that in-memory filesystem to follow the extraction process.I have no opinion on how this behaviour is to be triggered. It can be through the configuration file, the command line or by noticing that the entry file is a
.ts
file (not a.d.ts
). Obviously the current behaviour would stay the default.Motivation
In the wild, a lot of projects don't use the typescript compiler directly. Some use webpack loaders, rollup plugins, babel or ts-node, but specific tools are irrelevant to the discussion.
In this situation there can be either of two cases:
tsc
out-of-band on a published directory (the Libraries use case)Applications
Applications don't usually generate declarations files. However they may still want to use api-extractor as it provides the facilities to automatically generate an internal documentation.
This is how applications that use third-party loaders or transpilers such as the one listed above would do it currently:
tsc
(with for exampleemitDeclarationOnly
) so that it generates declaration files on atemp
directoryapi-extractor run
with the entry intemp
temp
directory (and all the other unused artifacts of api-extractor such asdist/tsdoc-metadata.json
)This is quite involved. With the proposed change, this is how they would do it:
api-extractor run
with the entry as their main.ts
fileMuch simpler.
Libraries
TypeScript libraries already generate declaration files. But they may not want to if the two following conditions are met:
Without api-extractor,
tsc
is called anyway (probably withemitDeclarationOnly
) and the relevant.d.ts
file is exposed intypings
. However, with api-extractor and dts rollup, the process would look like this:tsc
(with for exampleemitDeclarationOnly
) so that it generates declaration files on atemp
directoryapi-extractor run
with the entry intemp
(publishFolder
is set to be the published directory andtypings
inpackage.json
points to the generated.d.ts
file)temp
directoryThis is the same burden as with the first use case. With the proposed change, this is how they would do it:
api-extractor run
with the entry as their main.ts
file (publishFolder
is set to be the published directory andtypings
inpackage.json
points to the generated.d.ts
file)That's it! Just one step, like before.
Making a Pull Request
If you approve this proposal I'd like to try a PR.
This is what makes sense to me but I suppose that if typescript exposes the proper data structure and if we are able to resolve references through it an actual in-memory filesystem would be overkill.
The text was updated successfully, but these errors were encountered: