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

[WIP] Builtin declaration merging #7919

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

goodmind
Copy link
Contributor

@goodmind goodmind commented Jul 11, 2019

Somewhat (sometimes) working fix for #396

@goodmind goodmind changed the title Libs override [WIP] Libs override Jul 11, 2019
@goodmind goodmind changed the title [WIP] Libs override [WIP] Declaration merging Jul 11, 2019
@goodmind goodmind changed the title [WIP] Declaration merging (WIP) Declaration merging Jul 11, 2019
@goodmind goodmind changed the title (WIP) Declaration merging [WIP] Declaration merging Jul 11, 2019
@goodmind goodmind changed the title [WIP] Declaration merging [WIP] Builtin declaration merging Jul 16, 2019
@goodmind goodmind added the declarations Issues with library definitions or .js.flow features label Jul 16, 2019
@tonyn4444
Copy link

Nice!

@goodmind
Copy link
Contributor Author

Not really, I'm not sure this is right way to do this, but it works at least

@kevinbarabash
Copy link

@goodmind any plans to eventually add declaration merging for declare module eventually?

@goodmind
Copy link
Contributor Author

@kevinbarabash I'm not even sure if I'm implementing it right way, still wait on review from Flow team

@goodmind
Copy link
Contributor Author

goodmind commented Jul 22, 2019

@kevinbarabash just implemented declare module it was pretty easy actually, but it supports only ES declare export from modules, not private interfaces

Also you can't do augmentation in the same file, Flow would complain about re-declaring identifier

The thing you lose with this implementation is ability to completely override class with your own types, it would always merge them into original one

Also you can't really do anything with merging .js.flow files because they are orthogonal to declare module

@kevinbarabash
Copy link

I noticed you added GlobalThisT in this PR. It seems independent of the declaration merging. globalThis seems really hard to type properly given that it can have different properties depending on execution context.

@goodmind
Copy link
Contributor Author

goodmind commented Aug 8, 2019

Ah, because I was working on top of my globalThis PR,
I noticed that I can do globalThis.HTMLElement = 1; and it would make union of class HTMLElement | number

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed declarations Issues with library definitions or .js.flow features Experiment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants