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] Emit /// <reference lib="" /> in rolled up .d.ts #922

Closed
southpolesteve opened this issue Nov 1, 2018 · 7 comments
Closed
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change

Comments

@southpolesteve
Copy link

I'm attempting to adopt API extractor in https://github.com/Azure/azure-cosmos-js and ran into a blocker. /// <reference lib="" /> directives are missing from the rolled up .d.ts file. We had switched to these recently instead of specifying lib in tsconfig.json to make things more pleasant for TS consumers.

Relevant Azure Cosmos issue Azure/azure-cosmos-js#161
Relevant TS issue microsoft/TypeScript#26497

@octogonz
Copy link
Collaborator

octogonz commented Nov 1, 2018

@southpolesteve I'd be happy to help you get this working. Would you be able to provide a Git branch with an isolated example that repros the issue? That would help me to investigate it more easily. If this is a time-sensitive need, feel free to contact me at work.

@octogonz octogonz added the needs more info We can't proceed because we need a better repro or an answer to a question label Nov 1, 2018
@southpolesteve
Copy link
Author

southpolesteve commented Nov 1, 2018

@pgonzal Here is my WIP branch https://github.com/Azure/azure-cosmos-js/tree/api-extractor

npm run compile && npm run extract-api

@octogonz octogonz added enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start! effort: easy Probably a quick fix. Want to contribute? :-) effort: medium Needs a somewhat experienced developer and removed needs more info We can't proceed because we need a better repro or an answer to a question effort: easy Probably a quick fix. Want to contribute? :-) help wanted If you're looking to contribute, this issue is a good place to start! labels Nov 12, 2018
@octogonz
Copy link
Collaborator

Thanks @southpolesteve for providing the repro branch.

We're currently using the 3.0.x version of the TypeScript compiler. It seems that the compiler itself does not emit the /// <reference lib="____" /> into the .d.ts files unless I upgrade to 3.1. So the first step is to upgrade API Extractor to use the newer compiler. That's pretty easy.

The second challenge is that TypeChecker.getSymbolAtLocation() seems to skip over the /// <reference... line and take us directly to the declaration in the typescript/lib folder. In other words, it's not obvious how we would determine which reference lines we need to include in the rollup file. We could cheat and simply include every reference from every source file that we touch. That might cause problems if it's overly inclusive, however. I'm out of office today, but tomorrow I will chat with the compiler owners and see if there's an API that would allow us to determine this precisely.

It occurred to me that maybe API Extractor should provide a config setting that allows you to manually inject text into your rollup file. This could provide a fairly general workaround when people encounter edge cases like this.

@octogonz
Copy link
Collaborator

@iclanton Could we set up a 3.1 flavor of rush-stack-compiler?

@octogonz
Copy link
Collaborator

octogonz commented Nov 12, 2018

We could cheat and simply include every reference from every source file that we touch. That might cause problems if it's overly inclusive, however.

I noticed that we're already doing something similar with typeReferenceDirectives (for /// <reference type="___" />) in DtsRollupGenerator._collectTypeDefinitionReferences(), and it doesn't seem to have caused any problems so far.

The compiler API provides a similar libReferenceDirectives array, so if there isn't a more elegant solution, we can just collect them in the same way:

    interface SourceFile extends Declaration {
        typeReferenceDirectives: ReadonlyArray<FileReference>;
        libReferenceDirectives: ReadonlyArray<FileReference>;
        . . .
    }

@octogonz octogonz changed the title Emit /// <reference lib="" /> in rolled up .d.ts [api-extractor] Emit /// <reference lib="" /> in rolled up .d.ts Nov 16, 2018
@octogonz
Copy link
Collaborator

@southpolesteve This fix was published as API Extractor 6.1.6

@southpolesteve
Copy link
Author

This is looking good. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change
Projects
None yet
Development

No branches or pull requests

2 participants