Skip to content

Can't emit types into same namespace as values #146

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

Open
evmar opened this issue May 19, 2016 · 7 comments
Open

Can't emit types into same namespace as values #146

evmar opened this issue May 19, 2016 · 7 comments

Comments

@evmar
Copy link
Contributor

evmar commented May 19, 2016

interface Foo { x: number; }
var Foo;

is legal TS. But if you translate the interface into Closure directly it goes into the single (value) namespace and the two definitions conflict.

It's easy enough to just emit interfaces under some mangled name in Closure, e.g.

interface Foo { x: number }
/** @record */
function tsickle_Foo() {}
/** @type {number}
tsickle_Foo.prototype.x;

but then you need to fix every reference to that interface in any Closure type, while at the same time leaving other references (e.g. Array) alone. And you need to figure out how to import it across files (if a file does import {Foo} from bar, you'd need to change it to import {Foo, tsickle_Foo} from bar).

@evmar
Copy link
Contributor Author

evmar commented May 19, 2016

Rado suggested we might be able to do some trickery like

interface ts_Foo { ... }
type Foo = ts_Foo;

and then when Foo is used, note that there's an alias and inspect that to get data about the emitted type.

But a local experiment found that the place where we emit types gets the type after aliasing has been resolved.

evmar added a commit that referenced this issue May 20, 2016
Summary:
When we're not using Closure types, we don't need to emit Closure
interfaces.

Avoids #146, unblocks the tsickle sync into Google.

Reviewers: rkirov

Reviewed By: rkirov

Subscribers: typescript-eng

Differential Revision: https://reviews.angular.io/D120
@evmar
Copy link
Contributor Author

evmar commented May 23, 2016

Idea: always emit TS interfaces into a mangled Closure name, like tsickle_$name. Then there's no collisions possible, and it's always easy to guess the mangled name -- whenever you see an interface reference, emit it with the mangled name.

Problem 1. This breaks for builtin types like "Array" and "Object". However, emitting those interfaces verbatim as we do right now is saying that they are equivalent between TS and Closure's definitions of those terms. I think this is fine for Array and Object, but it might not be fine for more complex types. I think a whitelist (perhaps derived from lib.d.ts) of types that are considered compatible might get us pretty far.

Problem 2. Mangling names breaks interop with Closure code. E.g. a function returns a FooBar, and we see that and emit it as a tsickle_FooBar in our code.

I think collisions are unlikely enough that this approach is not gonna work. I will leave it here as a rejected proposal.

@evmar
Copy link
Contributor Author

evmar commented May 23, 2016

Idea 2: locally detect when there is a naming conflict and just dodge it there.

It might be possible to look up, given a symbol, whether that symbol represents both a type and a name in a TS file. When that scenario occurs, rewrite all the instances of the type to something else when emitting.

The hard part then is making the dependent code recognize it. Within a compilation unit perhaps we can just construct a map by doing one pass over all the files first before emitting any. Across compilation units, we can stash the mangled name into a comment and extract it from the docstring in the d.ts file(!).

This might actually work. I'm gonna prototype it now.

@evmar
Copy link
Contributor Author

evmar commented May 24, 2016

It took me quite a while to figure out how to find the conflicting symbols, but it seems to basically work in a single file test case. Next, I'll attempt a multi-file case.

Multi-file is hard because you can write:

import {Foo} from './bar';

and we need to figure out that it is

import {Foo, tsickle_Foo} from './bar';

(where tsickle_Foo is the interface name).

@evmar
Copy link
Contributor Author

evmar commented May 26, 2016

This works! At least in my test case -- it generates these funky names and adds extra exports/imports where necessary. But then it fails due to #112 . I need to fix that first before I can fix this.

@evmar
Copy link
Contributor Author

evmar commented Jun 14, 2016

Another simpler idea is: when there's a type/value conflict, just omit bringing the type into the Closure and treat it as {?}. That "solves" this problem at the cost of slightly weaker typing.

I still think I can implement the renaming thing but it might be worth checking how much it helps because it adds a lot of complexity.

@evmar evmar changed the title Can't emit interfaces into same namespace as values Can't emit types into same namespace as values Aug 20, 2019
@evmar
Copy link
Contributor Author

evmar commented Sep 17, 2019

We now think that #146 (comment) can work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant