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

assoc returns a narrower type than what is passed #35

Closed
ericyd opened this issue May 24, 2023 · 4 comments · Fixed by #100
Closed

assoc returns a narrower type than what is passed #35

ericyd opened this issue May 24, 2023 · 4 comments · Fixed by #100

Comments

@ericyd
Copy link

ericyd commented May 24, 2023

Summary

assoc sometimes returns a more narrow type than what is passed to it. Since assoc never removes props, this feels wrong from a type perspective. I would expect assoc to only add or modify props, never remove.

In the example below, a complex type with 2 props is typed as Record<'flag', bool> after passing through assoc

Dependencies

  • ramda@0.29.0
  • @types/ramda@0.29.1
  • typescript@5.0.2

Example

import { assoc } from "ramda";

type OriginalType = {
  name: string;
  flag: boolean;
};

function takesAnOriginalType(original: OriginalType) {
  console.log(original);
}

const array: OriginalType[] = [{ name: "test", flag: true }];

const mapped = array.map(assoc("flag", false));

takesAnOriginalType(mapped);
                    ^^^^^^

Error

const mapped: Record<"flag", boolean>[]
Argument of type 'Record<"flag", boolean>[]' is not assignable to parameter of type 'OriginalType'.
  Type 'Record<"flag", boolean>[]' is missing the following properties from type 'OriginalType': name, flagts(2345)
@Harris-Miller
Copy link
Collaborator

I've actually been working on this but had yet to push the branch up. I made a draft MR for you: #37

I still have a bunch of testing to do for the different overloads, but it does solve your issue

Using your example: https://tsplay.dev/mqxg2N

@ericyd
Copy link
Author

ericyd commented May 25, 2023

Wow, impressive - fixed the issue before it was even opened!! 👏🏻 👏🏻 👏🏻

Thank you for the diligent and speedy work!

@Harris-Miller
Copy link
Collaborator

@ericyd The original MR that I referenced to fix your issue had Breaking changes and I was forced to leave hanging since we're still debating on if we want to merge it or not

Prompted by another issue, I just put up a new MR that is non-breaking but also fixes your issue: #100

Sorry for letting this issue hang for so long

@ericyd
Copy link
Author

ericyd commented Feb 22, 2024

Wow I just re-read my original bug report and the code snippet I provided is not even an accurate representation of the issue 🤣

Anyway, no worries on the delay. As things go, I was laid off from the job where I used ramda a bunch and now my new company uses lodash 🙄

Thanks for your dedication to this project 🙇🏻 Ramda is great and these types make it even better!

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 a pull request may close this issue.

2 participants