Rewrite TakeTwo, SkipTwo and Mutate to make them work for older ts versions too
#1348
Conversation
...to work with both pre 4.3.5 and post 4.3.5 subtyping rules. The type `Test` in the following snippet resolves to `"yes"` for <=4.3.5 but `"no"` for >4.3.5 ```ts type Test = [string, number?] extends [unknown] ? "yes" : "no" ``` (note 4.3.5 is bisected via typescript playground which skips some versions so it might not be super accurate, but it's accurate enough)
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0997ee5:
|
|
Also having two + "tsc": "./node_modules/typescript/bin/tsc",
- "pretest": "tsc --noEmit",
+ "pretest": "yarn tsc --noEmit",Let me know if there's a better way. Also if you want to remove the this test and testing strategy then I'm fine with that too, but I think it's nice to have it. |
Haha thanks.
Sure. If you want that test to run only on ci, then I think there are several ways to do, probably the simplest way would be to use "test": "jest",
"test:ci": "jest",
- "test:dev": "jest --watch --no-coverage",
+ "test:dev": "jest --watch --no-coverage --testPathIgnorePatterns ci-only", |
Ha interesting. But unfortunately this won't work because ther's an error in the source too... If you want to test with this approach you'll either have to add an redundant I actually like my testing strategy more, we're only testing what we specifically want to test and support. This is a bit overboard as in we'll have to keep in mind old versions too while authoring. But again I'm fine with whatever. |
Yeah, I notice that too. But, then it means that this PR doesn't fix for older ts versions, doesn't it? |
Kinda yes, but that type error won't happen for the user as the user only has |
That's my intention actually. |
Ha, well if you want to take supporting older versions seriously then it's not a bad idea :P |
|
So, it seems like our lowest supported ts version is v4.1.5. @devanshj Do you know how to fix the issue in v4.4.4? |
...wrt to the domain `StoreMutatorIdentifier[]` to help the type-checker
devanshj
force-pushed
the
fix-issue-1013
branch
from
e5075f1 to
fbb2a14
Compare
Let me explain this a bit, imagine we're writing this code... const join =
<L extends string[], D extends string>
(l: L, d: D) =>
l.join(d) as Join<L, D>
type Join<L extends string[], D extends string> =
L extends []
? "" :
L extends [infer H extends string]
? H :
L extends [infer H extends string, ...infer T extends string[]]
? `${H}${D}${Join<T, D>}` :
never
let x = join(["a", "b"] as ["a", "b"], " | ")
// typeof x is "a | b"Often when we write types like type Test0 = Join<["a", "b"], " | ">
// "a | b"
type Test1 = Join<"a"[], " | ">
// never, should be string
type Test2 = Join<string[], " | ">
// never, should be stringThe last two arguments are indeed in the domain let x: "a"[] = ["a", "a"]
let test1 = join(x, " | ")
// typeof test1 is never, should be string
let y: string[] = ["a", "b"]
let test2 = join(y, " | ")
// typeof test2 is never, should be stringSo a more complete version of type Join<L extends string[], D extends string> =
number extends L["length"]
? string :
L extends []
? "" :
L extends [infer H extends string]
? H :
L extends [infer H extends string, ...infer T extends string[]]
? `${H}${D}${Join<T, D>}` :
neverNow the result will always be Now one might ask the question... But what if we never pass arrays to The thing is even if we don't pass tuples, typescript does... How? Let's look at the following code... const join =
<L extends string[], D extends string>
(l: L, d: D) =>
l.join(d) as Join<L, D>
const identitiyString =
<T extends string>(t: T) => t
const join2 =
<L extends string[], D extends string>
(l: L, d: D) =>
identitiyString(join(l, d))
type Join<L extends string[], D extends string> =
L extends []
? "" :
L extends [infer H extends string]
? H :
L extends [infer H extends string, ...infer T extends string[]]
? `${H}${D}${Join<T, D>}` :
number
let x = join2(["a", "b"] as ["a", "b"], " | "))
// typeof x is "a | b"Here one would assume that the only thing that ever gets passed to const join =
<L extends string[], D extends string>
(l: L, d: D) =>
l.join(d) as Join<L, D>
const identitiyString =
<T extends string>(t: T) => t
const join2 =
<L extends string[], D extends string>
(l: L, d: D) =>
identitiyString(join(l, d))
// ~~~~~~~~~~[1]
type Join<L extends string[], D extends string> =
L extends []
? "" :
L extends [infer H extends string]
? H :
L extends [infer H extends string, ...infer T extends string[]]
? `${H}${D}${Join<T, D>}` :
number
let x = identitiyString(join(["a", "b"] as ["a", "b"], " | "))
// typeof x is "a | b"
/*
[1]:
Argument of type 'Join<L, D>' is not assignable to parameter of type 'string'.
Type 'string | (L extends [infer H extends string, ...infer T extends string[]] ? `${H}${D}${Join<T, D>}` : number)' is not assignable to type 'string'.
Type 'L extends [infer H extends string, ...infer T extends string[]] ? `${H}${D}${Join<T, D>}` : number' is not assignable to type 'string'.
Type 'number | `${string}${D}${number}`' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
*/Ah ha, the fact that it reached to So same happens with our
(This is the same error in the previous CI in context.test.tsx, typescript seems to report this error only once if the origin is same, because we tested on the build there was no implementation where typescript could encounter it, so the first time it encountered it as in context.test.tsx, but it's the same error.) Here while type-checking our code, it must have found the need to find out what The code type-checks in the newer version probably because the compiler must have gotten smarter and either didn't had the need to pass So to help the type-checker we make this change to export type Mutate<S, Ms> =
+ number extends Ms['length' & keyof Ms] ? S :
+ // don't mutate if we get an array, it's just to aid the type-checker
+ // and except that it'll never happen
Ms extends [] ? S :
Ms extends [[infer Mi, infer Ma], ...infer Mrs]
? Mutate<StoreMutators<S, Ma>[Mi & StoreMutatorIdentifier], Mrs> :
neverNow |
devanshj
changed the title
TakeTwo and SkipTwo to make them work for older ts versions tooTakeTwo, SkipTwo and Mutate to make them work for older ts versions too
|
Wow, what "a bit". Thanks for the fix. This is great! |
Hahahah, little did I know when I started writing the explanation, how much of "a bit" it'll be xD
Nps and thanks! |
Fixes #1013. Superceeds #1275.
What is the issue?
The core of the issue is this: TypeScript changed subtyping rules of tuples with optional elements (probably) in v4.4.4. The type
Testin the following snippet resolves to"yes"in v4.3.5 and before but it resolves to"no"in 4.4.4 and after.This should most probably be intentional as the change is so old and 4.4.4 also happens to be the version that came with
--exactOptionalPropertiesso it's possible it touched upon tuples with optional element. It's also possible that this is a "fix" or that it increases the "strictness" of the subtyping rules. (Also turning--exactOptionalPropertiesoff or on does not have any effect on the subtyping.)Now how does it affect us? We have a type
TakeTwo<T>that does a type-level equivalent of...Now the problem is
TakeTwodoesn't work for pre-4.4.4 version...So now even though our code is "correct" it doesn't work with pre-4.4.4. Same is the case for
SkipTwothat does a type-level.slice(2).How do we solve it?
Now in some sense we can't write a code that works for both versions because typescript changed the math itself.
Imagine you have a function
fasconst f = (a, b) => a <> b. Now in wild-land-4.3<>did subtraction, but now in wild-lang-4.4<>does addition. Now you can't write a functionfthat does addition in both wild-lang-4.3 and wild-land-4.4.So if we want to write
TakeTwothat works for both pre-change and post-change versions, we'll have to use only those rules that are same in both, some subtyping rules for tuples with optional element are not same so we can't use them.What is consistent in both is the length property, eg
[string, number?]["length"]is1 | 2for both versions. So we can compare the lengths instead of comparing the tuples themselves...So the new
TakeTwolooks like this...(The last three cases remain the same as the subtyping rule is consistent for them, and nor there's another way of writing them.)
And we make a similar change for
SkipTwoalso.How do we test it?
Now usually libraries only target the latest typescript version, or whatever version the repo is running on, but here we have to target an old version.
This usualy would have been simple and we could test it with a script like so...
But now the problem is the version 4.3.5 is so old that there would be some current tests that would be failing because we're using new features, eg
--exactOptionalProperties. So now those type-check errors are expected, so we can't just rely on the exit code. What we'll have to do see if the tests type-check except some errors that we expect.To do this I'm using inline snapshots. That is the
oldTsc --noEmitoutput should only contain type errors we expect.To confirm that this change in
TakeTwoandSkipTwofixes the problem, see the snapshot in first commit, it has 38 errors, but now after we make this change, the new snapshot in the second commit only has 6 expected errors that we can't fix because it's an older tsc that doesn't support the new features and changes.