Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Alias colliding names in Closure Library like goog.ui.Component #811

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

teppeis
Copy link
Contributor

@teppeis teppeis commented Dec 12, 2018

Closure Library causes has 18 errors of TS2417 like the following.

TS2417: Class static side 'typeof Container' incorrectly extends base class static side 'typeof Component'.
  Types of property 'EventType' are incompatible.
    Type 'typeof EventType' is missing the following properties from type 'typeof EventType': ACTION, ACTIVATE, BEFORE_SHOW, BLUR, and 17 more.

https://github.com/teppeis/closure-library.d.ts/blob/master/errors.txt

In ES (TypeScript) classes, static props of classes are inherited to subclasses.
But in Closure, props of constructor function are not inherited.
For example, goog.ui.Component.EventType is a prop of goog.ui.Component, but it's goog.provide'd by itself and not inherited to the subclasses.

This PR aliases classes that has such props and causes the errors.
By this change, the props are only imported directory, but not available as static props of classes.

// Good
import EventType from 'goog:goog.ui.Component.EventType';
console.log(EventType);

// Bad
import Component from 'goog:goog.ui.Component';
console.log(Component.EventType);

@teppeis
Copy link
Contributor Author

teppeis commented Dec 13, 2018

Rebased on latest master

Copy link
Contributor

@rkirov rkirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

Interestingly, enough the whitelist used in Google, does not contain those files. Likely we don't have much goog.ui usage with TypeScript these days.

Copy link
Contributor

@rkirov rkirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looking at the original issue - 39ca7c2

This workaround should be only needed if the two goog.provides are in separate files (thus two separate --partial invocations). Otherwise, I think the code should emit the workaround based on the local knowledge, and no need for using the whitelist.

In fact, I took your cl and removed the list and your test - goog_ui_component, still produced code that is valid TS. In TS one can merge a class and namespace

declare namespace a {
  class b {...}
}
declare namespace a.b {
  let x: number;
}

Maybe I am misunderstanding the issue you are working around?

@teppeis
Copy link
Contributor Author

teppeis commented Dec 20, 2018

@rkirov

In fact, I took your cl and removed the list and your test - goog_ui_component, still produced code that is valid TS.

No. This is the generated code and is not valid.

In --partial mode, this problem cannot be resolved on the local knowledge, so whitelist is needed. (How can I refer to extends ಠ_ಠ.clutz.goog.ui.Component__clutz_alias from other file?)

In non-partial mode, I can guess it to a certain extent (1: is a shadowed symbol, 2: is a constructor, 3: has an enum property that is goog.provide'd), but I do not know whether it is correct for all users, so I think that it is better to limit the target area using whitelist. That makes the code simpler.

@teppeis teppeis force-pushed the colliding-closure-library-names branch from 8a7c2fa to d81d0ea Compare December 20, 2018 14:32
@teppeis teppeis force-pushed the colliding-closure-library-names branch from d81d0ea to eb25274 Compare January 19, 2019 03:27
@teppeis
Copy link
Contributor Author

teppeis commented Jan 19, 2019

rebased

@teppeis teppeis force-pushed the colliding-closure-library-names branch from eb25274 to 61546ca Compare January 30, 2019 17:55
@teppeis teppeis force-pushed the colliding-closure-library-names branch from 61546ca to e56656b Compare February 23, 2019 02:39
@teppeis teppeis force-pushed the colliding-closure-library-names branch from e56656b to ba4f88e Compare March 5, 2019 00:04
@teppeis teppeis force-pushed the colliding-closure-library-names branch from ba4f88e to 1bef4d6 Compare March 14, 2019 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants