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

Symbol type is not filtered out of keys in types for fromPairs, keys, toPairs, and toPairsIn #104

Open
MatthewTFarley opened this issue Mar 1, 2024 · 2 comments

Comments

@MatthewTFarley
Copy link

Problem

#50 incorrectly altered the original types for fromPairs, keys, toPairs and toPairsIn.

The implementation of these functions filter out symbol keys via for ... in or Object.keys. The original types captured this fact. The altered types do not. This has implications for operations that work with the outputs of these functions that expect symbol key types to be excluded.

Sandbox Demo: https://stackblitz.com/edit/vitejs-vite-tqnrhv?file=src%2Framda-demo.ts&terminal=dev

Implementation References

@Harris-Miller
Copy link
Collaborator

Great catch! Symbols() are inherently non-enumerable. Do you have a solution in mind? If not I can look into fixing

@Harris-Miller
Copy link
Collaborator

While looking into this, I found out it's worse than just symbols. The current typing for toPairs includes any non-enumerable property in its output, including class methods!

Check out this simple instance of a class put through R.toPairs

Now let's put the same code into typescript/play

Simplified here, you get this

import * as R from 'ramda';

class Point {
  public x: number;
  public y: number;

  constructor(x: number, y: number) {
    this.x = x;
    this.y = y;
  }

  map(fn: (a: number) => number) {
    return new Point(fn(this.x), fn(this.y));
  }
}

const point = new Point(3, 2);

const asPairs = R.toPairs(point);
//    ^? ["x", number] | ["y", number] | ["map", (fn: (a: number) => number) => Point]
// but the value you get is `[["x", 3], ["y", 4]]`

What this means is we can never use keyof to determine what should be part of the output array of key/value pairs

Because you also would have map if you use a function to create and object literal:

const makePoint = (x, y) => ({
  x,
  y,
  map: fn => makePoint(fn(x), fn(y))
});

const point2 = makePoint(3, 2);
const asPairs2 = R.toPairs(point2);
//    ^? ["x", number] | ["y", number] | ["map", (fn: (a: number) => number) => Point]
// but the value you get is `[["x", 3], ["y", 4], ["map", null]]`

I've always thought that the way Object.entires was typed was just due to very old typescript, before it had the ability to be more specific

entries<T>(o: { [s: string]: T; } | ArrayLike<T>): [string, T][];

But I understand now that it's due to how keyof does not descriminate against non-enumerables and non-own-properties

I'm thinking that the types for toPairs and fromPairs just needs to be typed exactly like Object.entries and Object.fromEntries, respectively.

Same with Object.keys and R.keys, Object.values and R.values, etc

Shitty thing is this is going to be a breaking change

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

No branches or pull requests

2 participants