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

Potential for better map support in Object.values / Object.entries? #2174

Closed
motiz88 opened this issue Aug 2, 2016 · 22 comments
Closed

Potential for better map support in Object.values / Object.entries? #2174

motiz88 opened this issue Aug 2, 2016 · 22 comments

Comments

@motiz88
Copy link
Contributor

motiz88 commented Aug 2, 2016

What we have today:

// in Object:
static entries(object: any): Array<[string, mixed]>;
static values(object: any): Array<mixed>;
// ...
let o: {[key: string]: number};
// ...
for (const v of Object.values(o)) {
  console.log(Math.abs(v));
//                     ^ v: mixed, incompatible with number
}

What may be better in the special case where object is used strictly as a map:
(Note: I'm being deliberately cautious, which is also why this is not a PR 😄)

static values<T>(object: {[key: string]: T}): Array<T>;
static entries<T>(object: {[key: string]: T}): Array<[string, T]>;

I tried out these declarations locally over the library ones, and while I was worried about breaking something in the heterogenous case (objects like {a: 1, b: 'b'}), Flow actually seemed to infer a valid T from whatever I threw at it, and carried on splendidly.

So, unless I'm missing something, I think these can become the new declarations of values and entries.

@avikchaudhuri
Copy link
Contributor

Thanks for the suggestion! @samwgoldman WDYT?

@Jessidhia
Copy link

Jessidhia commented Aug 3, 2016

Yes, it would be very nice if Object.values no longer erases the types. Extra nice would be if it yielded a union of all of the values' types (e.g. number | string for an object with only number and string keys).

The problem is when the object escapes; e.g. is given as an argument to a function that is not known to not mutate the argument, or is put in a variable that can be accessed from a different function scope. It becomes harder to guarantee that the object didn't get assigned an enumerable own property with a value of a different type.

@samwgoldman
Copy link
Member

I want to improve the situation very much. Unfortunately, the solution isn't quite so simple.

function f(o: { p: string }): Array<string> {
  return Object.values(o);
}
f({ p: "", q: 0 });

We can have a more specific type for exact objects, but due to width subtyping not for annotations (except the new $Exact<T> type, for which this is a motivating use) and objects with a dictionary component, as all own properties are described by the dictionary.

It would not be hard to build a CustomFunT (like Object$Keys) that handles exact objects and objects-as-map exactly but non-exact objects conservatively, however.

@dchambers
Copy link

Sorry for the noob question, but what's the recommended work around in cases where the author knows that the array won't be mutated and wants to use Object.values() anyway?

The best I've managed so far is this:

global.objectValues = (map) => Object.values(map);
declare function objectValues<T>(object: {[key: string]: T}): Array<T>;

but had originally anticipated being able to just use a typecast or something?

@kevinbarabash
Copy link

kevinbarabash commented Nov 26, 2016

Object.keys style iteration type checks fine, but Object.entries doesn't. I don't understand the distinction. In both cases something could've mutated attrs.

function mutate(attrs) {
    attrs.date = new Date();
}

function setAttributes(elem: HTMLElement, attrs: {[key: string]: number | string}) {
    mutate(attrs);
    Object.keys(attrs).forEach((key) => {
        const value = attrs[key];
        elem.setAttribute(key, `${value}`);  // everything is fine
    });
    for (const [key, value] of Object.entries(attrs)) {
        elem.setAttribute(key, `${value}`);  // 'value' cannot be coerced to string
    }
}

@samwgoldman
Copy link
Member

Sorry for the very long delay here. This thread is long overdue for an update.

One issue is exactness. An object type { p: number } is valid for an object { p: 0, q: 0 }. Returning just Array<"p"> from Object.keys leads to unsoundness because the array actually includes "p" and "q". Luckily, we have a solution for this with exact object types.

Another issue is that these APIs only work on "own" and "enumerable" properties. Flow does not currently track these attributes of object properties precisely.

For example, An object type { p: T } is valid for some value o if o or any object in its prototype chain have a property p of type T. That means we don't know if p is own or not, so we don't know if it should appear in the result.

We don't have anything like exact object types for own-ness, but we are experimenting with a solution: exact object types should also imply that all properties are "own." This is actually pretty reasonable to do, because even the simple typing judgment ({ p: 0 } : {| p: number |}) isn't quite "true" because {p: 0} inherits properties from the Object prototype like toString which means it is wider than just {| p: number |}.

With that in hand, if we enforce that an object type {| p: T |} is only valid for some value o if o has only one own property, p, then we can give a precise type for Object.keys(o) and .values and .entries. We can also implement object spread precisely, which is an added bonus.

There are a few more details to work out, but that's the plan for now. Sorry to keep you all waiting for so long without an update.

@montogeek
Copy link
Contributor

Thanks @samwgoldman for the update :)

@ghost
Copy link

ghost commented Jan 22, 2017

Thanks for the update! Is the best current solution to cast through any back to the proper type?

@callum
Copy link

callum commented Apr 11, 2017

@samwgoldman what's the latest on this?

@rmvermeulen
Copy link

bump

@freddi301
Copy link

@samwgoldman finally got $Values in 0.50.0 🎆

@rvion
Copy link

rvion commented Sep 1, 2017

I made it work like that :

function getValues <Obj:Object>(o: Obj) : Array<$Values<Obj>> {
    return Object.values(o)
}

see this comment: #4771 (comment)

@benjamine
Copy link

benjamine commented Sep 3, 2017

a flow-typed version of Object.entries:

function entries<T>(obj : { [string]: T }) : Array<[string, T]> {
  const keys : string[] = Object.keys(obj);
  return keys.map(key => [ key, obj[key] ]);
};

@jamesmfriedman
Copy link

Found a rather trivial way around this. Just just infer the type and then recast it.

const renderedQuestions = Object.entries(questions)
  .map(([questionId, questionValue]: [ID, *]) => {
    const question: QuestionT = (questionValue: any);
    // everything works as expected from here on out
    return ();
  });

@massimonewsuk
Copy link

massimonewsuk commented Jan 26, 2018

This is what we're doing about this (for now):

declare class Object {
    static entries<TValue>(object: { [key: string]: TValue }): [string, TValue][];
    static entries<TValue>(object: { [key: number]: TValue }): [string, TValue][];
}

@deecewan
Copy link
Contributor

deecewan commented Feb 1, 2018

@samwgoldman a year on, any update on better types for entries and values?
The main thing I'm confused about is the case where an object is declared as type Obj = { [string]: T }. In this instance, I feel like values and entries can have very simplistic signatures. I understand why this won't work with other objects, but I don't understand why this works perfectly well with Object.keys(...).map as shown here.

I don't know if this works (I'm not sure the precedence on overloaded definitions), but could we not leave the existing definition in place, and just put:

static entries<T>(object: { [string]: T }): Array<[string, T]>;
static values<T>(object: { [string]: T }): Array<T>;

into the libdef as well?

@TrySound
Copy link
Contributor

TrySound commented Feb 1, 2018

@deecewan Yep #5663 (comment)

@noppa
Copy link
Contributor

noppa commented Dec 18, 2020

If anyone's looking for a good alternative to the native function with better typing, I'm using this monstrosity that works pretty well

opaque type Unassignable = mixed

/**
 * Like the builtin Object.entries function, but uses a typed object
 * as the return values instead of tuples to get Flow working with it better.
 */
export function objectEntries<+T: any>(
  obj: T
): $NonMaybeType<$Values<
  $ObjMapi<T,
    <Key, Val>(Key, Val) => {
      +key: Key,
      +val: Val,
      // This is used to prevent accidental indexer access, e.g. entry[0]
      -[mixed]: Unassignable,
    }>
>>[] {
  return Object.entries(obj).map(toObjectEntry)
}

function toObjectEntry([key, val]: [string, any]) {
  return {key, val}
}

for (const entry of objectEntries({a: 'a', b: 2})) {
  if (entry.key === 'a') {
    entry.val.toUpperCase() // entry.val is inferred to be string here
  } else {
    entry.val.toFixed(2) // and number here
    entry.val.foo // Error as expected
  }
}

This is still unsound with inexact objects, but we aren't using them much anyway so 🤷

@deadalnix
Copy link

Considering the number of issue linked, this seems to be a recurring pain point. Is there a plan to address this?

@gkz
Copy link
Member

gkz commented Feb 16, 2022

Considering the number of issue linked, this seems to be a recurring pain point. Is there a plan to address this?

It's a known issue and something we would like to address at some point. However, we have limited resources and other projects have priority at the moment.

@gkz
Copy link
Member

gkz commented Dec 5, 2022

With above commit, object types that have an indexer and no explicit properties (e.g. {[string]: number}) have the type of their dictionary values returned by Object.values (e.g. Array<number>), and similarly with Object.entries, except the first element of the tuple works exactly like Object.keys (in this case, Array<[string, number]>)

@deadalnix
Copy link

Super cool! Thanks @gkz !

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