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] Naming conflict between import and global type renames global #1316

Closed
rudolf opened this issue Jun 6, 2019 · 4 comments · Fixed by #1767
Closed

[api-extractor] Naming conflict between import and global type renames global #1316

rudolf opened this issue Jun 6, 2019 · 4 comments · Fixed by #1767
Labels
bug Something isn't working as intended effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start!

Comments

@rudolf
Copy link
Contributor

rudolf commented Jun 6, 2019

If there's two type references, one a named import, the other a global variable, api-extractor renames the global variable instead of renaming the named import.

Example:

import {React} from 'react';
export interface UsingNamedImport {
  children: React.ReactNode;
}

export interface UsingGlobal {
  children: React_2.ReactNode
}
@octogonz octogonz added bug Something isn't working as intended effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start! labels Jun 6, 2019
@octogonz
Copy link
Collaborator

octogonz commented Jun 6, 2019

The problem is with _collectGlobalNames() which is used by _makeUniqueNames():
https://github.com/microsoft/web-build-tools/blob/d061216478054c8e2011bc3f4a1954b165f48264/apps/api-extractor/src/collector/Collector.ts#L386-L397

The above code aims to detect all names that are defined in the global namespace, but at the time we were unsure how to implement it. (This is tracked by #1095.) The proposal was to use internal compiler APIs like this:

const typeChecker = (this.program as any).getDiagnosticsProducingTypeChecker();
const resolver = (typeChecker as any).getEmitResolver();
resolver.hasGlobalName('Promise'); // --> true

However, the comments say that this produced 1,650 results which is a fairly large set that is likely to have unnecessary matches: For example, if we implement it that way, it would fix @skaapgif's example above. But the imported React would still get renamed even if the React global variable is not used.

Thus perhaps _collectGlobalNames() should /only/ return the global names that are actually referenced somewhere in the Collector's API surface.

@rbuckton Do you happen to know if there's an API similar to resolver.hasGlobalName() that can check a symbol to see whether it is referring to a global variable?

@rbuckton
Copy link
Member

rbuckton commented Jun 6, 2019

There's no public API for this currently, but a workaround is possible using checker.getSymbolsInScope

@octogonz
Copy link
Collaborator

Fixed by PR #1767 which was published with API Extractor 7.7.12.

@octogonz
Copy link
Collaborator

BTW I've also improved --diagnostics switch to report when globals are encountered, which will show the path of which file is referencing the React global.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants