Rationalize type/value conflict handling. #1126
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While separate in TypeScript, in Closure Compiler the type and value namespaces are merged. This means tsickle must take care not to emit duplicate symbols for type/value conflicts, and chooses the value over the type. That in turn means tsickle cannot generally emit references to types that have a type value conflict, i.e. symbols in the TS API that have both the Type and the Value SymbolFlag set. However we special case two situations: (1) builtin symbols such as `Array`, where we rely on the Closure definition and thus assume the type/value conflict is handled on that level; and (2) symbols generated by Clutz, which are also defined in Closure land so the conflict is not a problem. However previously we had two different code paths both handling parts of this, one for type references in `@implements`, one for all other type references. Both paths only handled either (1) or (2). This change rationalizes the situation, handling (1) and (2) in both code paths.
While `implements Boolean` might be not particularly sensical, this asserts that we call the right functionality for type/value inheritance conflicts.
These are included in the TypeScript standard library, but not in the Closure Compiler externs.
blickly
approved these changes
Dec 4, 2019
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While separate in TypeScript, in Closure Compiler the type and value
namespaces are merged. This means tsickle must take care not to emit
duplicate symbols for type/value conflicts, and chooses the value over
the type.
That in turn means tsickle cannot generally emit references to types
that have a type value conflict, i.e. symbols in the TS API that have
both the Type and the Value SymbolFlag set.
However we special case two situations: (1) builtin symbols such as
Array, where we rely on the Closure definition and thus assume thetype/value conflict is handled on that level; and (2) symbols generated
by Clutz, which are also defined in Closure land so the conflict is not
a problem.
However previously we had two different code paths both handling parts
of this, one for type references in
@implements, one for all othertype references. Both paths only handled either (1) or (2). This change
rationalizes the situation, handling (1) and (2) in both code paths.
Fixes #1116.