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

Custom dynamic type tests? #3048

Closed
bluepnume opened this issue Dec 20, 2016 · 11 comments

Comments

@bluepnume
Copy link

@bluepnume bluepnume commented Dec 20, 2016

Say I have the following code, everything works:

function isGreaterThan5(x : string | number) {
    
    if (typeof x === 'string') {
        return parseInt(x) > 5;
    }
    
    return x > 5;
}

Because flow's dynamic type tests recognize the typeof check.

However -- if I refactor this code slightly and break out the typeof check, it fails:

function isString(y) {
  return typeof y === 'string';
}

function isGreaterThan5(x : string | number) {
    
    if (isString(x)) {
        return parseInt(x) > 5;
    }
    
    return x > 5;
}

Is it possible to somehow mark my isString function as a pure-function which validates for a particular type? Like

function isString(y) { /* typecheck string */
  return typeof y === 'string';
}

or something?

@bluepnume

This comment has been minimized.

Copy link
Author

@bluepnume bluepnume commented Dec 20, 2016

Another reason this seems necessary is for instanceof checks, which can give unexpected results when type checking objects from a different frame: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_context_(e.g._frames_or_windows)

As such it's sometimes necessary to abstract these checks out into helper functions, which given the above, Flow doesn't seem to respect...

@awto

This comment has been minimized.

Copy link

@awto awto commented Dec 20, 2016

this is actually Occurrence Typing feature, implemented for example in Typed Scheme and Typed Clojure, it would unlock really a lot if implemented, here is an example in Scheme guide http://www.cs.utah.edu/~mflatt/tmp/trash/doc/ts-guide/occurrence-typing.html

really interesting if it is planned to be implemented in flow or not

@vkurchatkin

This comment has been minimized.

Copy link
Contributor

@vkurchatkin vkurchatkin commented Dec 20, 2016

Flow has this already:

function isString(y): %checks {
  return typeof y === 'string';
}

It's experimental at the moment, though. See also #34

@bluepnume

This comment has been minimized.

Copy link
Author

@bluepnume bluepnume commented Dec 21, 2016

@vkurchatkin Thanks, this is perfect!

One problem I had with this was, flow seems to still use built-in type tests for functions marked with %checks -- so while I could do:

function isNodeList(item) : boolean %checks {
  return item instanceof NodeList;
}

I couldn't do:

function isNodeList(item) : boolean %checks {
  return item instanceof NodeList || Object.prototype.toString.call(item) === '[object NodeList]';
}

Without losing the NodeList refinement.

Looked at the spec though, and it turns out you can declare a %checks function with a condition -- which makes everything works fine and solves the problem of typechecking across different frames, where instanceof wouldn't work:

declare function isNodeList(item: mixed): boolean %checks (item instanceof NodeList);

function isNodeList(item) : boolean {
  return item instanceof NodeList || Object.prototype.toString.call(item) === '[object NodeList]';
}

function getFirstParentNode(collection : mixed) : ?Node {
  
  if (isNodeList(collection) && collection.length) {
    return collection[0].parentNode;
  }
}

It would be cool if we could inline this though, rather than needing a declare... like so:

function isNodeList(item) : boolean %checks (item instanceof NodeList) {
  return item instanceof NodeList || Object.prototype.toString.call(item) === '[object NodeList]';
}
@vkurchatkin

This comment has been minimized.

Copy link
Contributor

@vkurchatkin vkurchatkin commented Dec 21, 2016

@bluepnume you can't do this because it's incorrect: Object.prototype.toString.call(item) === '[object NodeList]' doesn't prove anything.

@bluepnume

This comment has been minimized.

Copy link
Author

@bluepnume bluepnume commented Dec 21, 2016

Why doesn't it prove anything? The only scenario I can think of is someone monkey-patched Object.prototype.toString. And there are plenty of other scenarios where it's impossible for flow to check for monkey-patching. Someone could monkey patch Array.isArray, flow would pass, but the code would fail at runtime.

@vkurchatkin

This comment has been minimized.

Copy link
Contributor

@vkurchatkin vkurchatkin commented Dec 21, 2016

Like this:

const obj = {
  [Symbol.toStringTag]: 'NodeList'
};

You assume that NodeList is a part of DOM API, but other environments can also have class with the same name. Anyway, that point is that sometimes Flow is really strict about such things, so you have to opt-in by using .js.flow files or any

@bluepnume

This comment has been minimized.

Copy link
Author

@bluepnume bluepnume commented Dec 21, 2016

Sure -- but other than duck-typing, this is really the only way I know of to reasonably reliably check types on a different frame/window.

Also, surely limiting myself to:

  • Instances of NodeList
  • Instances on other windows of NodeList
  • Objects with [Symbol.toStringTag]: 'NodeList'

Is still preferable to using any? It's not perfect but it gets me 90% of the way there, and gives me a degree of static type safety.

Unless you have any other ideas for making flow work with cross-frame objects/types?

@vkurchatkin

This comment has been minimized.

Copy link
Contributor

@vkurchatkin vkurchatkin commented Dec 22, 2016

I'd say, yes. It might be good enough for you, but not for somebody else. any is just a way to tell Flow that you know what you are doing

@istarkov

This comment has been minimized.

Copy link

@istarkov istarkov commented Feb 7, 2017

Guys, how to run this function isNodeList(item) : boolean %checks { with node (babel-node)
as all that %checks gives unexpected token errors for javascript.

PS: Thank you @vkurchatkin for /* :: %checks */
solved!

@mtnt

This comment has been minimized.

Copy link

@mtnt mtnt commented May 14, 2018

I got an error "Flow: Unexpected token %checks" using %checks (value === null) of /* :: %checks (value === null) */

Is it actual syntax?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.