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

Composition of Lenses do not type properly #714

Closed
marcoemrich opened this issue Jan 2, 2024 · 8 comments
Closed

Composition of Lenses do not type properly #714

marcoemrich opened this issue Jan 2, 2024 · 8 comments
Assignees

Comments

@marcoemrich
Copy link

marcoemrich commented Jan 2, 2024

Issue

Composing functions of type R.Lens do not provide a proper Lens-Type.

Example:

const quantityLens = R.lensProp('quantity')
const composedLens = R.compose(quantityLens)
const result1 = R.view(quantityLens, {quantity: 5}) // this works
const result2 = R.view(composedLens, { quantity: 5 }) // this results in TS error (see screenshot)

screenshot_lens-issue

One-Liner to demo the issue

const composedLens: R.Lens = R.compose(R.lensProp('foo')) 

Error:
Property 'set' is missing in type '(obj: unknown) => unknown' but required in type 'Lens'.ts(2741)

Maybe this is expected behavior. But it does work in Ramda - in that case, consider it a feature request instead of a bug.

Rambda version

8.6.0

Node.js version

20.6.1

@selfrefactor
Copy link
Owner

I will sync with Ramda types and this should be fixed. I will post update as soon as I have it.

@selfrefactor
Copy link
Owner

selfrefactor commented Jan 3, 2024

I will need also your TS version. I did test with latest TS and this is what I see:

2

not sure how to proceed further

@marcoemrich
Copy link
Author

TS-Version is 5.3.3

But isn't the import in the screenshot above coming from Ramda instead of Rambda?

@selfrefactor
Copy link
Owner

I wanted to first check how your example works in Ramda, so that I can aim for similar result

@marcoemrich
Copy link
Author

The Ramda version needs the generics type info to work

import * as R from 'ramda'

const quantityLens = R.lensProp<{ quantity: number }, 'quantity'>('quantity')
const composedLens = R.compose(quantityLens)
const result1 = R.view(quantityLens, { quantity: 5 })
const result2 = R.view(composedLens, { quantity: 5 })

@marcoemrich
Copy link
Author

The fact that rambda does not need the annotation <{ quantity: number }, 'quantity'>, I would consider one of the big advantages of rambda over ramda

@selfrefactor
Copy link
Owner

I used your latest example with Rambda code. It is not great as result1 is unknown, which led me to decision to change Rambda types with ramda.

If you have better suggestion how to fix this without this big change, I am open to your feedback.

As I don't use lenses that much, I am not the best source for the correctness of their types, so I will do the fastest solution which is to sync.

selfrefactor pushed a commit that referenced this issue Jan 4, 2024
selfrefactor added a commit that referenced this issue Jan 16, 2024
* split for.each

* prepare x release

* chore@small

* chore@small

* chore@small

* chore@small

* chore@small

* fix@issue #710

* add gte

* chore@small

* chore@small

* add typings

* start reduce by

* chore@small

* fix@issue #714 sync types

* chore@small

* feat@inner.join

* feat@build

* fix test

* chore@small

* chore@small

* chore@small

* chore@small

* chore@small

* chore@small

* chore@small

* feat@fix

* chore@small

* chore@small

---------

Co-authored-by: Deyan Totev <deyan.totev.extern@seven.one>
@selfrefactor
Copy link
Owner

released with 9.0.0
this issue is the reason for the breaking change as types of lenses are synced with ramda

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

2 participants