flow

flow copied to clipboard

The window global is still typed as any. This means you can't get type coverage for window.addEventListener (see #2255) and many other things like window.location, window.history, etc. Many of these work fine when accessed directly as globals, but many people prefer not to do that, e.g. Airbnb's very popular style guide prohibits all the globals in this list to guard against cross-platform surprises, requiring you to access them via e.g. window.location, which Flow types as any despite having a good location type available.

Is there any reason we can't update Flow's global window declaration to something like declare var window: Window; and declare a Window class with all the right props?

I am happy to make a PR, if others think the above is OK in principle.

---

Related (in case I make a PR): are there any guidelines for how to choose the best syntax/style for typing globals etc? There seem to be multiple approachs in bom.js that may be interchangeable in many cases:

// declare a global type
declare type IntersectionObserverOptions = { /* ... */ };

// declare a global class and a global singleton instance
declare class Location { /* ... */ }
declare var location: Location;

// declare global var with a direct, un-named object type
declare var NodeFilter: { /* ... */ };

// declare interface -- never used in bom.js (why?)

Also would it be good to include [string]: any as a final prop on the window type, to allow for stuff that creates ad hoc globals?

Is there any reason we can't update Flow's global window declaration to something like declare var window: Window; and declare a Window class with all the right props?

Not that I know of.

are there any guidelines for how to choose the best syntax/style for typing globals etc?

The first one doesn't actually declare the existence of a window object, just the type, so you'd still want to declare var window: Window.

The last one is essentially a shortcut for the first one + declaring the var, but I'd suggest against it since it means there's a name for the variable (In this case NodeFilter) but not for the actual type since it's all declared literally.

So it really comes down to:

// Option A
declare type Window = { /* ... */ };
// Option B
declare class Window { /* ... */ }
// + this to actually define the global window object
declare var window: Window;

Now, the difference between these two is that A defines an object type (what you'd get if you made an object literal) while B defines a class which you can have extend things. If you look up the prototype chain for window for example, you'd see it extends EventTarget:

Well, technically it extends WindowProperties. I don't think you have to worry about the WindowProperties superclass, since it doesn't seem to have any own properties really, and I don't really know if it's used anywhere else? So you can probably just extend EventTarget directly. After that EventTarget extends Object, and Object is the superclass for all objects so it extends nothing.

Anyway, EventTarget is defined here:

https://github.com/facebook/flow/blob/1dc257c64e6777f952c44d60229c15da1cb0fa2d/lib/dom.js#L158

So you could declare it window as:

declare class Window extends EventTarget { /* ... */ }
declare var window: Window;

Also would it be good to include [string]: any as a final prop on the window type, to allow for stuff that creates ad hoc globals?

Yes.

I think Flow shouldn't include types for window or there should be some config for environmental libdefs

What about creating a dom.js or window.js which could be provided via https://github.com/flow-typed/flow-typed - at least as an interim until deemed solid enough for inclusion in Flow itself?

Just ran into myself, and it was fairly unexpected.

I imagine the reason that window is any-typed has to be related to the fact that any newly created globals get added to window automatically. This would certainly prove difficult to type correctly with flow. Whether or not any is the best solution is certainly debatable.

I'll note that this seems to be much less of an issue in Typescript, as the Window interface over there can just be re-opened and have properties added to it at any point, and flow doesn't really have any corresponding functionality.

This would be great to have. I think we haven't yet heard from a member of the Flow team in this thread, so quoting here the encouraging words of one of them (@nmote) on a Stack Overflow answer a couple of years ago:

It doesn't look like there is any particular reason, just that it hasn't yet been given a better type. You can see in the DOM libdef file that it is declared as returning any, and that line hasn't been touched in over four years.

This would be an easy first PR, if you are interested in improving it.

---

I imagine the reason that window is any-typed has to be related to the fact that any newly created globals get added to window automatically. This would certainly prove difficult to type correctly with flow. Whether or not any is the best solution is certainly debatable.

I think this can be handled with the following idea from the OP:

Also would it be good to include [string]: any as a final prop on the window type, to allow for stuff that creates ad hoc globals?

Here's a quick try-flow demonstrating that that works.

That still wouldn't get you specific type-checking on any newly created globals you add -- but you don't have that today with window: any, so it's still purely an improvement.

That makes sense. I may do some work on this over the next week if nobody else wants to.