WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@Harris-Miller
Copy link
Collaborator

Reverts #74

@Nemo108 In my MR out on , this comment pointed out a test that I had commented out. I had the intention of coming back to, but forgot (human error).

That test is this:

    const coll = [{ type: "BUY" }, { type: "SELL" }, { type: "BUY" }];
    const isBuy = R.propEq("BUY", "type");
    R.filter(isBuy, coll); // => [{ type: 'BUY' }, { type: 'BUY' }]

You get an error on isBuy being passed to R.filter:

Expand to see error ``` No overload matches this call. Overload 1 of 5, '(pred: (val: { type: string; }) => val is { type: string; }, list: readonly { type: string; }[]): { type: string; }[]', gave the following error. Argument of type '>>(obj: Required extends Record<"type", any> ? string extends WidenLiterals ? U : never : never) => boolean' is not assignable to parameter of type '(val: { type: string; }) => val is { type: string; }'. Signature '(obj: { type: string; }): boolean' must be a type predicate. Overload 2 of 5, '(pred: (val: unknown) => val is unknown, dict: Record): Record', gave the following error. Argument of type '>>(obj: Required extends Record<"type", any> ? string extends WidenLiterals ? U : never : never) => boolean' is not assignable to parameter of type '(val: unknown) => val is unknown'. Types of parameters 'obj' and 'val' are incompatible. Type 'unknown' is not assignable to type 'Partial>'. Overload 3 of 5, '(pred: (value: unknown) => boolean, collection: { type: string; }[]): { type: string; }[]', gave the following error. Argument of type '>>(obj: Required extends Record<"type", any> ? string extends WidenLiterals ? U : never : never) => boolean' is not assignable to parameter of type '(value: unknown) => boolean'. Types of parameters 'obj' and 'value' are incompatible. Type 'unknown' is not assignable to type 'Partial>'.ts(2769) ```
It's very long and hard to understand. tl;dr is that the function signature returned by `R.propEq("BUY", "type")` is not compatible as a first argument for `R.filter()`. Or in other words, typescript does it see it as a `(pred: (val) => boolean)`

I'm pretty sure it's because of how we have the turnary and the : never. The fact that that function's first argument could be the bottom type never, it will never be compatible with R.filter() or any function that is looking for a a function predicate such at that.

This means that #74 breaks existing behavior in a net-negative way. While it did solve a lot of the defined issues called out, I can't keep this as is.

This sucks, a lot. But I must revert it. I can't move forward if it's going to break consumer's existing code negatively like this.

@kurtinatlanta
Copy link

kurtinatlanta commented Mar 4, 2024

This is the second time in the last couple weeks that a change to the propEq typings has caused numerous downstream issues for me. Can we please stop messing with this in a patch release as it causes breakage downstream and is not by any means backward compatible.

Thanks.

@Harris-Miller
Copy link
Collaborator Author

@kurtinatlanta First, let me apologize for our typing updates giving you so much trouble. It's not our intention. I personally messed up with propEq. tl;dr is that how I normally test the develop branch against DefinitelyTyped before releasing gave me a false positive because of how they switched from npm to pnpm. So I released types-ramda with failing tests.

I don't have that flow captured anywhere, but I'll add it to the readme. I'm also going to add an issue to get some some Gitlab Actions created that adds testing directly against DefinitelyTyped

To address patch... Unfortunetly we're stuck with patch releases both here and for @types/ramda. This project and that one is not synced with core ramda. And we cannot diverge from major.minor. We can only diverge patch

eg. ramda is 0.29.1 currently, if we want to do type fix, we have to release 0.29.x always. We can't do 0.30.x until the core package bumps to 0.30.x as well. We have to keep those synced. This is true for all "community" typings, and is no isolated to us.

@kurtinatlanta
Copy link

Thanks. I get that stuff happens from time-to-time. I'm sure you'll get it all worked out.

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

Successfully merging this pull request may close these issues.

3 participants