-
Notifications
You must be signed in to change notification settings - Fork 64
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
flatten() is broken #356
Comments
It works fine with our types, it seems you're using types from DT instead of ours, see Usage. |
Alright, downloaded the new version and they're sort of better, but they still yield wrong output types or compile errors on valid, though admittedly less common, inputs. For instance: const x1 = R.flatten(['a', ['b'], [['c']]]); // Compile error
const x2 = R.flatten(['a', ['b'], [['c'], 'd']]); // Will be type (string | string[])[] (I guess it's a matter of policy whether to try to accept valid inputs at the cost of returning less-precise types or not. For my first example, I'd have to cast the input to and then the output to Array.) On a side note: Why does the repo listed in index.d.ts file for @types/ramda redirect to this repo, but this readme says nothing about the DT typings being so out of date/wrong? The name of this library in package.json is @types/ramda, yet the version in package.json of the latest dist is a lower version than what's in DT, and that's especially confusing for someone wanting the latest version. Also, why does this repo's usage section suggest installing from a Github branch, which will certainly change over time? Surely releases can be pushed to NPM, or at the very least, git tags can be made so we can get reproducible builds? (Yes, we could use commit hashes but that's not exactly a user-friendly solution.) |
We probably need Conditional Types (microsoft/TypeScript#12424) or The types in DT is a fork from this repo and it's maintained by DT these days, we're trying to get sync with it (#191) but I do not have time to work on it recently. |
@typings/ramda
"version": "0.25.8"
The type of
d
isstring[][]
as you'd expect. However the type ofy
is alsostring[][]
instead ofstring[]
., which is what the runtime type is.I know we can't really get the typings correct for all valid arguments, but I would at least expect the most basic use case to be correct, and I don't think the types currently returned are ever correct right now.
To fix this, I think something like this:
Then my first use case is fine, as are larger, but consistently-nested, arrays like
[[['abc'], ['def']], [['xyz']]]
. (You can of course add more overloads to allow higher nesting of arrays.) If you get anything more nested, irregularly nested, or have a mix of types, it'll default toany[]
, which isn't too helpful but at least isn't incorrect.The text was updated successfully, but these errors were encountered: