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

Skipping an @implements can cause misoptimization with typed optimizations #1072

Open
evmar opened this issue Sep 16, 2019 · 1 comment
Open
Assignees
Labels
typed optimizations Bugs that only cause problems with typed optimizations

Comments

@evmar
Copy link
Contributor

evmar commented Sep 16, 2019

In some cases we back off from emitting an "@implements" tag. But with typed optimizations, every "@implements" is potentially load-bearing.

At the TS level, every "implements" is always optional. At the Closure level, it is dangerous to ever leave out an "@implements". We have no good user-facing way of reconciling these two.

I think we should instead say something like: if you ever write "implements" in TS, we always emit an "@implements", and if the latter fails for some reason you should delete the "implements" from the TS side.

@evmar evmar self-assigned this Sep 16, 2019
@evmar evmar added the typed optimizations Bugs that only cause problems with typed optimizations label Sep 17, 2019
@evmar
Copy link
Contributor Author

evmar commented Sep 22, 2019

After some work in this area, I now better understand all the cases here.

  1. if the symbol comes from Clutz? then we should emit (<- this change)
  2. if the symbol is a TS builtin? then it probably exists in Clousure and we can emit it
  3. if the symbol comes from a user-written d.ts, then we can probably(?) emit it with name mangling
  4. if the symbol comes from a tsickle-transpiled file, then we can probably emit it with name mangling, possibly depending on whether namespaces are involved or not

Note that fixing #2 ends up triggering conformance because there's an Angular type that extends Window and by using that indirection people were calling unsafe window functions.

An approach to #4 that includes #2 are in #1077 but that was taking the wrong approach and triggered the problems in #2 and got #3 wrong I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typed optimizations Bugs that only cause problems with typed optimizations
Projects
None yet
Development

No branches or pull requests

1 participant