facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.08k stars 1.85k forks source link

way to extend/override flow/lib types #396

Open blax opened 9 years ago

blax commented 9 years ago

Built-in node.js type declarations are still far from being complete, and filling the missing parts is very cumbersome as Flow doesn't allow to overwrite single modules.

For instance, if I want to fill in some gaps in HTTP types, I have to use flow with --no-flowlib flag, which forces me to copy all the remaining built-in interfaces to my project.

Although such workaround works for now, it's unmaintainable in the longer term, and not obvious to newcomers.

Would it be a problem to make custom declarations override the default ones?

SethDavenport commented 6 years ago

It's also a bit awkward currently to adopt post-ES6 features - now that the spec is being updated more frequently there's perfectly reasonable stuff that either is standard or soon will be that's currently difficult to use.

For example: Promise.finally, currently stage 3.

I know that's not strictly official yet, but it will be soon. I'm not trying to extend a builtin; I'm just trying to keep up with an evolving language spec.

The babel team has done an excellent job with presets to make this type of stuff pluggable. It would be nice if flow allowed developers to keep up in a similar way.

gianpaj commented 6 years ago

Another vote for overwriting types:

For example, in mongoose when you create a database schema you define a number of fields. e.g. flow-typed/.../mongoose_v4.x.x/test_mongoose-v4.js#L32

but if you want to overwrite the _id field and not use this type:

_id: bson$ObjectId | string | number;

with something like this...

export class HashtagDoc /*:: extends Mongoose$Document */ {
  _id: string;
}

you can't:

Error: server/models/hashtag.model.js:16
 16:   _id: string;
            ^^^^^^ string. This type is incompatible with
270:   _id: bson$ObjectId | string | number;
            ^^^^^^^^^^^^^ bson$ObjectId. See lib: flow-typed/npm/mongoose_v4.x.x.js:270

Error: server/models/hashtag.model.js:16
 16:   _id: string;
            ^^^^^^ string. This type is incompatible with
270:   _id: bson$ObjectId | string | number;
                                     ^^^^^^ number. See lib: flow-typed/npm/mongoose_v4.x.x.js:270

Maybe a syntax like this would be nice?!:

export class HashtagDoc /*:: extends Mongoose$Document */ {
  _id: string!; // see the explamation mark ( ! )
}
qm3ster commented 6 years ago

@gianpaj I wouldn't call that "overriding" at all. Your new type is a strict subtype of the original. It should just work, and without any special syntax. It does precisely this in TypeScript.

vezaynk commented 6 years ago

Over 2 years down the line and still isn't addressed? 👎

ericketts commented 6 years ago

Presently have a great use case for this, as someone who used typescript more frequently in the past I was surprised to discover this ability absent from flow (declaration merging)

kvanbere commented 6 years ago

Yeah so we recently had to type a "plugin" for a package on NPM and because flow lacked this support ... we copy and pasted the entire types of the package into a separate flow-typed folder and had to modify it by hand :( took about 1 man day to troubleshoot and work around.

ericketts commented 6 years ago

if only i knew ocaml 😞

sobolevn commented 6 years ago

@calebmer @vkurchatkin could you please explain why this issue is marked as won't fixed?

I have a new use-case to discuss.

Reasoning

We try to write types for Vue. And we are not able to. See this pull request to vuejs/vue. Vue uses a method called .use() to install new plugins. That's how it works:

import Vue from 'vue'
import Vuex from 'vuex'

Vue.use(Vuex)
// now you can use `this.$store` inside components

We need to extend types here. Because there are plugins that provide lots of new fields: $http, $axios, $route, and many others. Since, there is no way to do it now. Which resolves in errors like:

error Cannot get app.$axios because property $axios is missing in Vue (see line 26)

Which is not true. That makes flow nearly unusable for this use-case. And that's a big project.

Possible solutions

There were many proposed solutions to this issue. I am not good enough with flow to suggest any of them to the core team.

Hidden issues/complexity

Are there any hidden issue why this issue can not be resolved? Maybe there are some philosophical reasons behind it?

My contribution

I can donate some money to the folks who would implement this. Let's say 100$. That's not much, but I am sure others will support me too.

hrgdavor commented 6 years ago

Just here to repeat my dissapointment with lack of interest for this by flow team.

mrkev commented 6 years ago

could you please explain why this issue is marked as won't fixed?

@sobolevn not sure, guessing it stems from the discussion at #4845

Are there any hidden issue why this issue can not be resolved?

After a 5 min look, none seem unresolvable, though it could potentially be a big code change.

Maybe there are some philosophical reasons behind it?

Idk, though it seems like it's just not a priority since there's technically a solution: you can use your own definitions instead of the ones provided. In theory you'd just need to copy flow/lib to your project and work from there, extending as you see fit. Has anyone checked if there's a higher quality definitions by third parties anywhere?

@hrgdavor I'm sorry to hear you're disappointed-- perhaps a good way to channel that frustration would be starting a good 3rd party libdef (:

sobolevn commented 6 years ago

@mrkev thanks for the detailed answer!

Idk, though it seems like it's just not a priority since there's technically a solution: you can use your own definitions instead of the ones provided. In theory you'd just need to copy flow/lib to your project and work from there, extending as you see fit.

It sound like a nightmare to me. I would personally prefer to have no types at all than to use some local, modified, outdated information.

After a 5 min look, none seem unresolvable, though it could potentially be a big code change.

Sounds promising. I really hope that this will be achieved. And we will have types for Vue. I am facing some much problems with this issue in my experiments: https://github.com/sobolevn/vue-flow-typed

Is there anything I can help with?

deecewan commented 6 years ago

For things like React, it'd be great to be able to extend the core types temporarily.

For example, react-native is currently at 0.55.4, which has Flow v0.67.1. It does, however, support React 16.3. I'd like to use createRef, which exists in Flow 0.72, but it's not available in 0.67. It'd be great to be able to extend the react module with a single definition for createRef instead of having to copy across the entire react definition from here.

syrnick commented 6 years ago

Not sure it's intentional, but this workaround seems to be working for me. Say, I have a case for process.myServiceId = process.env.MY_SERVICE_ID;. I added flow-typed/node.js with this:

class MyProcess extends Process {
    myServiceId: ?string;
}

declare var process: MyProcess;
STRML commented 6 years ago

Yeah, that works well for certain globals. But if you want to e.g. change the type of a field on EventTarget there's no good way that I know of to update it such that it will actually apply in all the many ways you might receive or pass an EventTarget.

pronebird commented 6 years ago

I am not able to use chai and chai-spies with Flow for that very reason. Just no way to tell that chai.use(spies) has side effects and which ones.

magus commented 6 years ago

For anyone coming into this looking for a solution, @nmn's post (https://github.com/facebook/flow/issues/396#issuecomment-125542040) earlier is now possible.

For example,

declare interface Document extends Document {
  mozHidden: boolean,
  msHidden: boolean,
  webkitHidden: boolean,
}
sobolevn commented 6 years ago

That still does not work with custom types.

declare interface Some {
  a: string
}

declare interface Some extends Some {
  b: string
}
5: declare interface Some extends Some {
                     ^ Cannot declare `Some` [1] because the name is already bound.
References:
1: declare interface Some {
                     ^ [1]

https://flow.org/try/#0CYUwxgNghgTiAEBLAdgFxDAZlMCDKA9gLYIDeAUPPFAFzwDOqMKA5uQL7nmiSwIrosOfMQQgAHumTB68QiXgUqAIzqNmyNpyA

AMongeMoreno commented 6 years ago

Any idea on when (if ever) flow will support extending existing modules? @deecewan makes a good point and now we are facing the same issue with forwardRef for react ...

sobolevn commented 6 years ago

I came up with the following solution to my Vue problem:

// @flow

import Vue from 'vue'

import type { Axios } from 'axios'
import type { Store } from 'vuex'
import type Router from 'vue-router'

import type { State } from '~/types/vuex'
import type { NuxtAuth } from '~/types/nuxt-auth'

/**
* Represents our extended Vue instance.
*
* We just use the annotations here, since properties are already injected.
* You will need to add new annotations in case you will extend Vue with new
* plugins.
*/
export default class CustomVue extends Vue {
  $auth: NuxtAuth
  $axios: Axios
  $router: Router
  $store: Store<State>
}

And then just using it like so:

// @flow

import Component from 'nuxt-class-component'

import CustomVue from '~/logics/custom-vue'

@Component()
export default class Index extends CustomVue {
  logout () {
    this.$auth.logout()  // type of `logout()` is defined in `NuxtAuth` 
  }
}

And it works!

AMongeMoreno commented 6 years ago

@sobolevn thank you for the suggestion ! The only 'drawback' is now you have to import 'custom-vue' instead of directly importing 'vue', which might (?) cause issues in plugins relying on the import of the actual library.

In my case, I had to deal with 'react' and I think JSX would not be properly understood if I would rename the 'react' library itself.

What I have ended up doing (in case someone else come to this post looking for the same) is to wrap the required function within a 'typed' version of it. Fortunately, in my case the function I was willing to add is forwardRef from React, which is already in the pipeline to be added, so I could refer to the proposed typing from https://github.com/facebook/flow/issues/6103.

In my case the definition looks like:

// $FlowFixMe
import {forwardRef} from 'react';
export function typedForwardRef<Props, ElementType: React$ElementType>(
    render: (props: Props, ref: React$Ref<ElementType>) => React$Node
): React$ComponentType<Props> {
    return forwardRef(render);
}
YaoHuiJi commented 6 years ago

I have meet this issue from the very first beginning of trying to introduce flow into my existing project,and this problem scares me away, and I do not like TypeScript too, so I think maybe I can only keep checking types using my eyes 🤷‍♂️

skeggse commented 5 years ago

I've got a related use-case: I'd like to force all interactions with DOM APIs to enforce sanitization of untrusted content. I can declare opaque type safeHTMLString = string;, and create APIs that accept a string, process it, and return a safeHTMLString. The missing ingredient is overriding the DOM APIs to only accept safeHTMLString and fail on string. For instance, document.querySelector('body').innerHTML = someStringValue; should fail.

gdanov commented 5 years ago

The only solution (actually dirty hack) advised so for (using --no-flowlib) does not work because the modules written this way can't be redistributed. I think the team owes some explanation to the public why so stubbornly they refuse to move the libs to flow-typed

sobolevn commented 5 years ago

Maybe the core team members have moved to TypeScript?

taybin commented 5 years ago

I found creating new types using intersections to be a decent way to extend existing, thirdparty types.

sijad commented 5 years ago

I was trying to use react hooks in react-native but RN only support flow 0.78 currently, so I can not upgrade flow-bin. I had to go with @AMongeMoreno solution.

moll commented 5 years ago

I'm looking to create some typings for a testing library of mine, Must.js, that extends Object and other builtins' prototypes. Am I correct in saying that this is still impossible to perform with Flow without copying the entirety of https://github.com/facebook/flow/blob/master/lib/core.js to Must.js and adding a property that way? I do see mentions of intersection types and type extensions, yet if the rest of the codebase is typed to return Object, it doesn't do much if there's also an ExtendedObject that no-one refers to. Thanks!

hrgdavor commented 4 years ago

Once again after more than a year I am looking into useful way to add strong typing ...

I just don't like TypesScript, and enabling intellisense for plain JS in VS code is not working properly yet ...... more than 4 years since this issue was opened, and nothing ...

leethree commented 4 years ago

Just try TypeScript. Evidently Microsoft cares more about community than Facebook.

unscriptable commented 4 years ago

It's now possible to override custom (user-land) types with the spread type operators. This is a huge win for me (finally!). Has anybody tried overriding built-in types this way?

unscriptable commented 4 years ago

Blog post: https://medium.com/flow-type/spreads-common-errors-fixes-9701012e9d58

TrySound commented 4 years ago

@unscriptable What do you mean by "override custom type"? Spread creates new type like values spread creates new object. There is no mutating happen.

volkanunsal commented 4 years ago

I think somebody needs to write a blog post about how spreads can be used in place of type extension. I can see a few use cases (listed in this thread) that I can’t figure out how a spread type would help. One way of formulating a motivating use case is this: Given a global type like MouseEvent, how might I use a spread so Flow doesn’t complain when I try to access a new property that is not yet added to the spec definition? And what if the MouseEvent is defined as a class rather than an object?