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

propIs #147

Closed
wants to merge 1 commit into from
Closed

propIs #147

wants to merge 1 commit into from

Conversation

fatsu
Copy link
Contributor

@fatsu fatsu commented Feb 27, 2017

I don't understand why all these definitions have been made so complex.

Let's take propIs as an example:
Looking at the ramda source code for propIs (https://github.com/ramda/ramda/blob/v0.23.0/src/propIs.js), the signature is defined as "Type -> String -> Object -> Boolean".

Why the generics? Why the Record? Why not simply return boolean? By making it so complex, restrictions and logic are added which the actual ramda library should be able to handle itself.

R.propIs(Number, 'x', {}); is a valid call returnning false, no error should be expected (https://github.com/ramda/ramda/blob/v0.23.0/test/propIs.js).

Maybe I see it too simple, but what is currently in this definition-file is way to complex.

@KiaraGrouwstra
Copy link
Member

KiaraGrouwstra commented Feb 27, 2017

Fair question. Thank you for asking. Short version, check type guards; just saying boolean is admittedly simple, but true vs. false can matter for inference in conditionals.

Maybe I see it too simple, but what is currently in this definition-file is way to complex.

TL;DR: I'd been trying to under all circumstances properly infer return types with maximum accuracy, which is hard, while doc types just aim to explain to humans. My reasoning is docs can take care of the explaining, while only the typings could help bring inference, so I'd say let each do what they do best. If they could be readable too, that'd be wonderful, but I'd consider it secondary -- and just that goal of inference has been challenging enough by itself. Right now I just wanna make our tests pass. That still seems hard enough as-is.

If you'd like, I actually have an even stronger example for you: consider the implementation of R.path versus that of its type. Now realize that this type definition is not only limited to covering certain path lengths, but has also been simplified such as to skip over the possibility of navigating over tuples (arrays containing values of more than a single type).

And we might be a bit exceptional in this respect -- you'll notice that Lodash's TS typings haven't been made remotely as complex. They're much closer to the doc-style Type -> String -> Object -> Boolean that you mentioned.

A short example of my view: what is the type of { a: 'b', c: 1 }? One might say Object. I would argue the type is exactly the value itself: { a: 'b', b: 1 }. You might wonder, why? Type inference.

The thing is, you may then navigate to the a property of this object, then do an operation on that. If you no longer knew the type of this property, type inference stops, you're back at any, and the compiler will have no way to prevent you from making mistakes.

Why say 'b' is the type there, rather than string? Again, it matters even on the type level: if you would then use this value to navigate this same object again, knowing the string value again enables it to figure out what kind of value you'd be getting there.
In short, type inference purposes would rather have you retain maximum information, which ideally includes even values.

Why not simply return boolean?

On this particular one, TS's type guard section provides some background.
Technically, functions could return something different depending on conditional logic using a boolean. If TypeScript knows beforehand whether this boolean would be true or false, it can help you infer what the function will return. So again, more accurate info helps TS help you.

Why the generics? Why the Record?
You're right. propIs is hella complex. In fact, I couldn't think of a proper solution -- the current idea I got from a TypeScript member after I got stuck.
As a quick answer though, the generics are needed because the result type (true or false) depends on the inputs (specifically: on whether the value at key of obj qualifies as type). Record, in this case, is used to express an object with the given key and value type, so as to check whether that matches with the input object (or, technically, array).

You noted

restrictions and logic are added which the actual ramda library should be able to handle itself.

You're not wrong. In fact, type definitions have no effect at run-time whatsoever -- everything is run by Ramda itself.

So the role of these typings isn't so much guiding Ramda at run-time, but rather guiding the user such as to write correct sane programs, telling them the result type, and calling out whenever the user has done something wrong in any chain of logic containing Ramda calls.

R.propIs(Number, 'x', {}); is a valid call returning false, no error should be expected

Right. Ramda will not error on this, just returning false, so this is a valid question.

My reasoning here has been not so much what Ramda has been doing, but rather what would be the most useful response here. In this particular case, we can see the value would never contain the checked property. In that case, I would wonder whether this call is useful. I'd be inclined to conclude the user has been making a mistake.

You may have a broader view on this, and I'm curious about counter-examples.
You'd wonder in what case I would consider this call useful. The way I'm thinking about this is, as long as there is a chance the value does contain the property, the call is potentially useful; if we are certain it would never contain it, it's starting to sound not useful, and if that's the case, we may need to notify the user of this potential logical flaw.

Does that line of reasoning resonate with you? Have you found cases where this errored for you in a context you did consider the call useful?

So, how did they end up this complex?

There's been a couple things:

  • The type as written in the docs was written with a completely different purpose in mind: instructing a user what a function does. By contrast, the TS typings have been written to enable proper (and precise) type inference.
  • The Lodash typings, which are closer to their docs, are relatively unambitious, only giving a vague description of what goes in and out, rather than taking the responsibility to help you infer when you've made some logical mistake a couple lines down from your Ramda call. I imagine they were written based on their docs, rather than to maximize inference, so that makes sense. There might be another aspect to that though: enabling proper inference in TS can be pretty damn tough, adding up to complex definitions, as you've noticed over here.
  • If you wanna know the actual return types for any given inputs / currying order, things get fairly complex, yet that's what I'd been trying to tackle. Ramda is quite flexible, with polymorphic functions (handling multiple types, using dispatching to enable yet others), and is just about all-curried too, which TypeScript as of yet doesn't have a great way to express. (Worse yet, FP support isn't high on the TS agenda.) Just currying already complicates things by an order of magnitude.
  • TS's type language doesn't come close to the expressiveness of the expression language. I wish TS could just follow the logic of the JS to infer the types, but it kinda sucks. It can't follow reduce, it doesn't know about currying, it doesn't yet have a way to capture generics within generics (-> composition/currying), and can't even retain types after a map operation unless the values in your array/object all had the same type. All in all, from the perspective of a typings repo for Ramda, the current state of TS is still a bit disappointing, and it definitely shows in our typings. In fact, as a workaround for the current inability to properly retain generics within interfaces (e.g. CurriedFunction2 and up), I've started to look toward further code generation to generate resulting curried typings for each function, which would eliminate current use of those CurriedFunction[n] interfaces but would clog up the type definitions even worse still. Why bother? Because the entire point of Ramda is doing partial application, and so far that just fails to survive currying type-wise for functions with generics (read: 90% of them).
  • Partial application just makes inference even worse. In order to infer return types for prop on an object containing values of different types, you'd need the likes of keyof to have it keep track of which key you're grabbing the value of. However, this requires knowing the type of the value you're taking a key of. In partial application however, this is not available, and you can only still get some inference in case all values of the object share the same type.
  • Worse, if curried versions of these definitions aren't combined into one interface, TypeScript will just give up after failing to find a match in the interface that seemed to match from the outside (e.g. first parameter).

You may wonder: might it be nicer to have TS typings optimized for instructing users?

Yeah. But I'd rather have it not detract from this first goal of just allowing users to write code and be told the result type when right, and get an error if wrong.

I'm sorry this ended up long, and I hope you'll sympathize on these considerations. That said, if there is demand for a version of the typings aimed at humans rather than inference, I'd be open to the idea of having a branch for that.

@fatsu
Copy link
Contributor Author

fatsu commented Feb 28, 2017

The context I'm using ramda in is for example ngrx/store reducer functions to handle Action={type:string, payload?:any} objects. These payloads are not explicitly typed.

I understand your urge to have complete type inference and I would appreciate it as well, but I still don't agree in this case.

Should a declaration file not describe the javascript library (as is) so it can be used in typescript code, and not add additional restrictions for what is clearly legal behaviour for this library.

When i try:
R.pipe(R.path(['payload', 'b', 'c']), R.propIs(Number, 'x'))(action)
I would in the first place expect it to return me a boolean value. Whether or not the returned value is the expected value is a point to be addressed by (unit) tests. Not by a compiler error.

@KiaraGrouwstra
Copy link
Member

Yeah, ngrx reducers functions are one of the use-cases most important to me personally as well.

I managed to reproduce your issue, and pushed a fix to address it now -- not throwing out the type inference, but rather just allowing fallbacks to catch cases like this.

The error was supposed to only trigger when it's actually known the key doesn't exist, but seems you're getting {} typing here due to generic degeneration, a TS problem that's our biggest issue right now, and one I'm currently working on solving using that code generation.

Your glitch affected any functions using keyof without fallbacks: pick, propIs, as well as uncurried versions of prop and propOr (fixed too to play it safe).

Thanks for reporting this. Definitely let me know in case you run into similar issues.

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.

2 participants