Skip to content

Wrong typescript declarations for .update() method (continued) #1931

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

Closed
cypherfunc opened this issue Feb 6, 2023 · 2 comments · Fixed by #1933
Closed

Wrong typescript declarations for .update() method (continued) #1931

cypherfunc opened this issue Feb 6, 2023 · 2 comments · Fixed by #1933

Comments

@cypherfunc
Copy link

What happened

This is a continuation of issue 1466.
In 1842, | undefined was added to the updater function's parameter type, but not the return type. As mentioned in a comment on that issue, the return type also needs to allow undefined. This is specifically needed for cases where no update is applied if the entry does not exist, or where the entry should be deleted.

e.g.

const myMap = Map<string, List<string>>();
const stillMyMap = myMap.update("noKey", ls => ls?.map(x => x + "blah"));

Since myMap doesn't have an entry for "noKey", ls is undefined, and thus the updater returns undefined.

How to reproduce

Run Typescript type-checking on the above example, with strictNullChecks turned on. You will get an error like:

error TS2345: Argument of type '(ls: List<string>|undefined) => List<string>|undefined' is not assignable to parameter of type  '(ls: List<string>|undefined) => List<string>'.
  Type 'List<string>|undefined' is not assignable to type 'List<string>'.
    Type 'undefined' is not assignable to type 'List<string>'.
@jdeniau
Copy link
Member

jdeniau commented Feb 7, 2023

@cypherfunc Can you try to patch the immutable definition file in #1933 to see if it does fix your issue before I merge ?

@cypherfunc
Copy link
Author

@jdeniau That's perfect, thanks so much! I didn't expect such a quick response.

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

Successfully merging a pull request may close this issue.

2 participants