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

Refine array types using filter #1414

Closed
vkurchatkin opened this issue Feb 17, 2016 · 35 comments
Closed

Refine array types using filter #1414

vkurchatkin opened this issue Feb 17, 2016 · 35 comments

Comments

@vkurchatkin
Copy link
Contributor

Not sure if it's possible, but it would be nice if flow could refine array type using filter:

function getArray(): Array<?{}> { [{}, {}, null, {}, null] }

const array: Array<{}> = getArray().filter(i => Boolean(i));

Simplest patterns that are already used to refine optional types would be just enough

@vkurchatkin vkurchatkin changed the title Refine array types Refine array types using filter Feb 17, 2016
@avikchaudhuri
Copy link
Contributor

Yeah, this feature should eventually be supported, along with the ability to annotate functions like the one you pass to filter with "effects." It's on the TODO list, but not hi-pri right now.

@vkurchatkin
Copy link
Contributor Author

Here is my proposal:

  • implement a magic type, that represents result of type check, for example Typecheck<Foo>;
  • Typecheck is subtype of boolean;
  • Typecheck is only allowed to be explicitly used as type of return value of a single-argument function:
function isNumber(val: mixed): Typecheck<number> {
  if (typeof val === 'number') {
    return true;
  }

  return false;
}
  • if typecheck function returns true, that means that argument is of type Foo;
  • typecheck function is validated, so you can only return true if val is of type Foo at this point;
  • typecheck function is only allowed to call other typecheck functions

For example, Boolean can be declared as <T: {}>(v: ?T): Typecheck<T>.

Finally, filter function could be declared like this:

filter<T2>(fn: (v: T) => Typecheck<T2>): Array<T2>

@ctrlplusb
Copy link

ctrlplusb commented Jul 11, 2016

I was just coming to post a similar issue. I am experiencing the following:

screenshot 2016-07-11 06 28 46

Completely kills my clean functional workflows... :'(

@ctrlplusb
Copy link

ctrlplusb commented Jul 11, 2016

@gcanti suggested the following workaround in #1915:

const input: Array<number|string> = [1, 'a', 2, 3, 'b']
const result = ((input.filter(el => {
  return typeof el == 'number'
}): Array<any>): Array<number>)

@ctrlplusb
Copy link

Sorry for the spam, but another update on this. Using .reduce seems to play much nicer with types. Just annotate your accumulator and it's all good.

@gabro
Copy link
Contributor

gabro commented Oct 4, 2016

It seems that with more recent versions of flow this is supported using a point-free style, e.g.

declare function getArray(): Array<?{}>
const array: Array<{}> = getArray().filter(Boolean);

The snippet above type-checks on flow.

https://flowtype.org/try/#0CYUwxgNghgTiAEAzArgOzAFwJYHtXwHMQMBBGGKATwAoBKALnjIsoB4B+AbwF8A+AKHjx+YPAGcM8WC0bMqrHr3gBeQsTk1aAOkRYIGEDGoAhHDgggoqWgG4gA

@vkurchatkin
Copy link
Contributor Author

It doesn't work for generic predicates though

@gabro
Copy link
Contributor

gabro commented Oct 4, 2016

Right, that one is still missing.

@lll000111
Copy link
Contributor

lll000111 commented Aug 28, 2017

Any progress?

By the way, I just found the ..filter(Boolean) doesn't even allow false, only null and undefined. Seems to be an oddly specific hack.

@bdrobinson
Copy link

+1 for this. In fact, since ?string is just a specific case of a union, it'd be great more generally if Flow could filter any type union: ie, filtering redux actions by type:

type SetNameAction = { type: "SET_NAME", name: string }
type SetAgeAction = { type: "SET_AGE", age: number }
type Action =  SetNameAction | SetAgeAction

const actions: Array<Action> = [] 

// currently this errors even though we can be certain that
// the array only contains `SetAgeAction`s
const setAgeActions: Array<SetAgeAction> = actions.filter(
    action => action.type === "SET_AGE")

Flow can already filter type unions with if/switch statements (which blew my mind the first time I saw it) – could a similar approach be applied to filter? (quite possibly not, I have no idea how these things work under the hood):

const printActionName = (action: Action) => {
  
  // flow correctly errors because action could be a SetAgeAction
  const name: string = action.name
  
  if (action.type === "SET_NAME") {
    // no error here as flow correctly infers that action must be a
    // SetNameAction from the if statement
    const name: string = action.name
    console.log(name)
  }
}

Thanks – flow is awesome!

@bradennapier
Copy link

bradennapier commented Nov 1, 2017

I run into quite a few situations where I am having to do value transformations whether it be filter or any other situation -- and while it isn't very difficult once you get the hang of it, I figured a little micro-package would be useful here to enforce type-safe value transformations in a standard manner.

Its late so I'll try to put together some nice examples that are actually useful soon -- for now there are a few documented and it should work very well.

It is published to npm as flow-type-transformer and is a runtime value transformer which is extremely tiny (500 bytes or so) and includes flow-type information for development.

Below is a completely useless example to illustrate the use. it simply provides a nice little transform utility type & transformation class:

import type { $Transform } from 'flow-type-transformer';
import createTransformer   from 'flow-type-transformer';

// pointless transformer example (since Flow can infer this)
const stringToNumber: $Transform<string, number> = createTransformer(str => Number(str))
const n = stringToNumber('1')

@ctrlplusb I would think something like this should make it far easier to build nice functional interfaces that remain type safe.

https://github.com/Dash-OS/flow-type-transformer

I tested against the array filter example (which arr.filter(Boolean) is a far better solution for in this case). It is obviously more verbose for the simple stuff but should make composing transformations
which are type safe fairly straight forward. It's already helped us quite a bit.

Below you will notice the transformer is not specifically doing what the $Transform utility type dictates. However, Flow will now type the specific instance it returns for us to make sure we are treating it as-such.

import type { $Transform } from 'flow-type-transformer';
import createTransformer from 'flow-type-transformer';

type BeforeType = Array<?string>;
type AfterType = Array<string>;

function removeFalseyFromArray<T>(v: Array<?T>): Array<$NonMaybeType<T>> {
  if (Array.isArray(v)) {
    return v.filter(el => el != null);
  }
  throw new TypeError('Value is not an Array');
}

const transform: $Transform<BeforeType, AfterType> = createTransformer(removeFalseyFromArray);

const before = ['one', undefined, 'two', null];

const after = transform(before);

// $Works
(before: BeforeType);

// $Works
(after: AfterType);

@jcready
Copy link
Contributor

jcready commented Nov 1, 2017

@bradennapier this might pass flow type checking, but it will also throw a javascript runtime error because a $call property doesn't actually have any significance in javascript in spite of what flow thinks. Running this results in an error (jsfiddle):

class TypeTransformer {
  constructor(transformer) {
    this.transformer = transformer;
  }
  $call(v) {
    return this.transformer(v);
  }
}

function createTypeTransformer(fn) {
  return new TypeTransformer(fn);
}

function removeFalseyFromArray(v) {
  if (Array.isArray(v)) {
    return v.filter(el => el != null);
  }
  throw new TypeError('Value is not an Array');
}

const transform = createTypeTransformer(removeFalseyFromArray);

const before = ['one', undefined, 'two', null];

const after = transform(before); // Uncaught TypeError: transform is not a function

@jcready
Copy link
Contributor

jcready commented Nov 1, 2017

However, you can make your class work as intended in actual javascript by changing the class to return the transformer from the constructor:

class TypeTransformer<Before, After> implements $Transform<Before, After> {
  transformer: $Transformer<Before, After>;
  constructor(transformer: $Transformer<Before, After>) {
    this.transformer = transformer;
    return transformer;
  }
  $call(v: Before): After {
    return this.transformer(v);
  }
}

This manages to trick flow into believing that calling new TypeTransformer(foo) returns an instance of TypeTransformer, but in actuality javascript is returning foo so now both flow and javascript are happy!

class Foo {
  constructor(bar) {
    return bar
  }
}

new Foo({}) instanceof Foo; // false

@bradennapier
Copy link

bradennapier commented Nov 1, 2017

@jcready yes you are correct. I had planned to change it to a standard function because at first the class was going to have things like .filter and map but then i had changed it to providing a custom transformer function.

However, your little workaround here is actually pretty perfect. I have made the adjustment! Thanks. I still think will need to consider a better way to keep this API the same while achieving needs as the instanceof check I would want to pass if possible.

Looks like I will need to do a little bit of modification to return something similar to how axios does things, which is where I copied the "$call" concept from.

https://github.com/axios/axios/blob/master/lib/axios.js

@bradennapier
Copy link

bradennapier commented Nov 1, 2017

So this has been really powerful for us already. For example, one thing that has always been a bit challenging is transforming object literal values.

Now we are able to do so using opaque types for a significantly stronger typing than normal and normalizing the shape in the process.

export opaque type Dash$ErrorResponse = {|
  result: 'error',
  code: number,
  uuid: string,
  path: string,
  errors: Array<string>,
|};

export const formatErrorResponse: $Transform<
  $Shape<{ ...Dash$ErrorResponse }>,
  Dash$ErrorResponse,
> = createTransformer(transformErrorResponse);

function transformErrorResponse(response) {
  if (!Array.isArray(response.errors) && typeof response.errorMessage !== 'string') {
    throw new TypeError('Error Response requires an errors array or an errorMessage');
  }
  const errors = response.errors || [];
  if (typeof response.errorMessage === 'string') {
    // If recieving the previous format for any reason,
    // convert to the new style array
    errors.push(response.errorMessage);
  }
  return {
    result: 'error',
    code: typeof response.code === 'number' ? response.code : 501,
    uuid: typeof response.uuid === 'string' ? response.uuid : '0',
    path: typeof response.path === 'string' ? response.path : undefined,
    errors,
  };
}

image

Flow really is starting to shine now. 100% coverage across our entire back-end.

FYI - Using $Shape<{ ...Type }> allows one to remove the "opaqueness" as seen by the caller while making the values optional (so that the transformer can handle these cases as desired).

@sibelius
Copy link

can we use $Pred and $Refine for this?

@Chaoste
Copy link

Chaoste commented Mar 5, 2019

I also struggled with this and it is a bit frustating that this is open for two years. The tip by @ctrlplusb to use reduce was very helpful!

So instead of my old code:

items.filter(item => item.type === MY_TYPE)

I'm now using:

items.reduce(
    (arr, item) => (item.type === MY_TYPE ? [...arr, item] : arr),
    []
);

Try Flow: forEach vs. reduce vs. filter

@virtue3
Copy link

virtue3 commented May 8, 2019

I also struggled with this and it is a bit frustating that this is open for two years. The tip by @ctrlplusb to use reduce was very helpful!

So instead of my old code:

items.filter(item => item.type === MY_TYPE)

I'm now using:

items.reduce(
    (arr, item) => (item.type === MY_TYPE ? [...arr, item] : arr),
    []
);

Try Flow: forEach vs. reduce vs. filter

Finally dealing with this myself and quite frustrated that I can't utilize filter very well. I feel like a major tool in my belt is now missing :(

I just wanted to point out that your solution was an excellent starting point and Ive iterated on it in order to take out some extra object creations with the splat operator (albeit more/uglier code):

items.reduce(
  (arr, item) => {
    if(item.type === MY_TYPE) {
      arr.push(item);
    }
   return arr;
 }
);

@Chaoste
Copy link

Chaoste commented May 14, 2019

@virtue3
Thanks for sharing your version!

I want to point out that the other workaround by @ctrlplusb is also quiet handy. It In my case I just needed to manually redefine the return value like this:

const result = (array.filter(MY_FILTER): Array<T>)

In my reducer, it was even easier to just flow type the return value of the superordinate function instead of using the above inline version:

const reducer = (state: MyArrayStateType = initialState, action: MyActionType): MyArrayStateType => {
    switch ...
        case ...
            return state.filter(MY_FILTER)
}

Same like in their own unit tests: https://github.com/facebook/flow/blob/41b0eab99cdc5199421f7cccad9e0c4950f8b2f9/tests/array-filter/test.js

However, it want to point out that this workaround only blocks flow from typing correctly. If the type is changing, you should use reduce. If it the same type as before but flow enforces you do add a type, you can do it like these two example above.

As you can see in this Try Flow, you can not use it for enforcing a new flow type.

@choonkending
Copy link

Given how widely we use filter, is it worth having a discussion on whether there is room to change the type definition itself Array's definition in Flow?

E.g., is something such as the following possible?

// Naive implementation
filter<FilteredType: T>(predicate: (value: T, index: number) => boolean): Array<FilteredType>;

Here is a Try Flow as a test case we wish to pass.

I'd love to not resort to reduce just to refine an array of values, as filter conveys a lot more in terms of readability in a large codebase rather than the powerful reduce.

tl;dr

I'd like to know:

  1. If refinement from A -> SubType of A is achievable with filter
  • not with the current definition of Array, but whether or not it is possible with Flow currently
  1. What we can do to move this forward + if any help is needed from us/anyone in the community

@goodmind
Copy link
Contributor

goodmind commented Jul 9, 2019

@choonkending

  1. $Pred and $Refine
  2. Well, nothing, it's still experimental features

@FlavienBusseuil
Copy link

In addition to what @bdrobinson said, I guess we have the same situation with find:

type Cat = { type: "cat" };
type Dog = { type: "dog" };
type Animal = Cat | Dog;

const cat: Cat = { type: "cat" };
const dog: Dog = { type: "dog" };
const animals: Array<Animal> = [cat, dog];

const aCat: ?Cat = animals.find(({ type }) => type === "cat");
// Cannot assign `animals.find(...)` to `aCat` because  string literal `dog` [1]
// is incompatible with  string literal `cat` [2] in property `type`.

@w01fgang
Copy link
Contributor

This issue looks quite old, but, at least for me, it's still actual.
From the definition of .filter it's clear that for now, the only way to filter is using .filter(Boolean).

filter(callbackfn: typeof Boolean): Array<$NonMaybeType<T>>;

Other methods are returning the same type as they receive.

filter(callbackfn: (value: T, index: number, array: Array<T>) => any, thisArg?: any): Array<T>;
find(callbackfn: (value: T, index: number, array: Array<T>) => any, thisArg?: any): T | void;

It seems there is a way to filter without using chainig.

type SubTypesOfString = 'A' | 'B';

declare function ABfilter<T, P: $Pred<1>>(v: Array<T>, cb: P): Array<$Refine<T,P,1>>;
function isValid(i): %checks {
  return i === 'A' || i === 'B';
}

const arr: Array<mixed> = ['A', 'C', 'D'];
const barr: Array<SubTypesOfString> = ABfilter(arr, isValid);

I found my way to make it work but for small arrays, because of additional iterations

const arr: Array<mixed> = ['A', 'C', 'D'];
const a: Array<SubTypesOfString> = arr
  .map(el => is_string(el) ? el : null)
  .filter(Boolean);

for the @FlavienBusseuil's example

const aCat: ?Cat = animals
  .map((el) => el.type === "cat" ? el : null)
  .filter(Boolean)
  .find(({ type }) => type === "cat");

try

@FlavienBusseuil
Copy link

@w01fgang thanks for your help.

Still I'm concerned by the boilerplates and performance issues added by those two loops.

Also I came up with another issue. Filtering based on object attribute presence:

type Cat = { meow: true };
type Dog = { woof: true };
type Animal = Cat | Dog;

const cat: Cat = { meow: true };
const dog: Dog = { woof: true };
const animals: Array<Animal> = [cat, dog];

const cats1: Array<Cat> = animals.filter((animal) => animal.meow);
// Cannot assign `animals.filter(...)` to `cats1` because property `meow` is 
// missing in `Dog` [1] but exists in `Cat` [2] in array element.

const cats2: Array<Cat> = animals
	.map((animal) => animal.meow ? animal : null)
	// Cannot assign `animals.map(...).filter(...)` to `cats2` because property 
	// `meow` is missing in `Dog` [1] but exists in `Cat` [2] in array element.
	.filter(Boolean);

const cats3: Array<Cat> = animals
	.reduce((cats, animal) => animal.meow ? [...cats, animal] : cats, []);
	// Cannot assign `animals.reduce(...)` to `cats3` because property `meow` is 
	// missing in `Dog` [1] but exists in `Cat` [2] in array element.

I really don't get this one... why animal.meow ? do not distinguish which type of object I'm working with? Am I missing something? 🤔

@noppa
Copy link
Contributor

noppa commented Jun 3, 2020

@FlavienBusseuil That problem goes away if you make the types exact.

type Cat = {| meow: true |};
type Dog = {| woof: true |};

without exact types, Dog could also have a meow property of some type, because extra properties are allowed, and the check would erroneously mark it as a Cat.

Maybe a helper function like this would help a bit with the loop cost and boilerplate.

@FlavienBusseuil
Copy link

@noppa thanks! As I moved with the new syntaxe without the pipes recently I totally forgot about them!

@w01fgang
Copy link
Contributor

w01fgang commented Jun 6, 2020

@noppa I found that there is a cast any and it doesn't work without it. In my codebase, I use flow strict preventing any types. I changed a bit your code replacing symbol by null, and it works! Thanks!
try

@noppa
Copy link
Contributor

noppa commented Jun 6, 2020

@w01fgang Yah the any cast there is a bit unfortunate but since it's quite a small implementation detail and fairly easy to see that it's safe, I'm fine with it. The problem with null is that then you can't have that as the expected array value type. But hey whatever works for your needs 👍
You could also use a class and instanceof (try), but that might come with a performance penalty compared to a simple === check.

@FezVrasta
Copy link
Contributor

FezVrasta commented Nov 3, 2020

I think I'm missing something, why is Array.prototype.filter so limited? Array.prototype.reduce works perfectly and it's basically a superset of filter, why can't filter be defined as a constrained reduce?

example:

Array.prototype.filter = function(callback) {
  return this.reduce((a, b, index, arr) => {
    if (callback(b, index, arr)) {
      return a.concat(b);
    } else {
      return a;
    }
  }, [])
};

@nnmrts
Copy link
Contributor

nnmrts commented Nov 9, 2020

@FezVrasta You're not missing anything. This has been a pattern for 2 years now. As @goodmind wonderfully pointed out when asked about "What we can do to move this forward + if any help is needed from us/anyone in the community" by @choonkending:

Well, nothing, it's still experimental features

I don't really know what "experimental" in this context means, but my guess is language-wise, which would be just plain wrong in the case of filter, which has been added to Javascript before Windows Vista was released.

But that wouldn't be the first time flow just didn't support a 15-year-old, core part of the ES standard because "it's unsound" or something like that. But actually, @goodmind has a point, you really shouldn't try do anything here, unless you want to waste your time.

But you do you, good luck and have fun! 💪

@noppa
Copy link
Contributor

noppa commented Nov 9, 2020

@nnmrts I think @goodmind was referring the experimental Flow features $Pred and $Refine there, that could've possibly been used to impement this feature in userland.

Based on this thread, though, those features are not going to become available for general use.

@FezVrasta
Copy link
Contributor

Is there any way to override the built-in definition without copying the whole built-in Flow definitions?

@lyleunderwood
Copy link
Contributor

@FezVrasta see #7919

@w01fgang
Copy link
Contributor

w01fgang commented Apr 12, 2022

Well, not really. It's just stupid typecasting.
Screenshot 2022-04-12 at 10 09 32

Filtering arrays based on types is such fundamental functionality, it leads one to believe development of Flow is basically dead. R.I.P Flow.

Also, your original message is quite offensive. TS is hardly pushed by MS while people love Flow and decide to use it because of its value.

-- Edit
The message to which this message is directed was deleted.

@SamChou19815
Copy link
Contributor

This now works with type guards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests