Redesign discriminated union type parser #2068
Conversation
…o have a simpler and more intuitive interface.
benchristel
requested review from
anakaren-rojas,
catandthemachines,
jeremywiebe and
nishasy
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (ccee41e) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2068If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2068 |
|
Size Change: +79 B (+0.01%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
| // Guard against implicit 'any' type | ||
| // @ts-expect-error - Type '{ shape: "circle"; radius: number; }' is not assignable to type 'string'. | ||
| parser satisfies Parser<string>; |
There was a problem hiding this comment.
I understand all the tests in the file except this one. Why would this potentially be implicit any? Or what does this test protect against?
There was a problem hiding this comment.
Sometimes, when you do complicated things with types, TypeScript can't figure out what the type of an expression is so it just infers any 😱 . I was worried that the type of parser would be inferred as any.
parser satisfies Parser<Figure>; asserts that parser is assignable to a variable of type Parser<Figure>;. This assertion will succeed if parser is actually a Parser<Figure>, but it will also succeed if parser has type any.
To guard against that case, I always include a negative assertion in my type tests as well: parser should not be assignable to Parser<string>. If the type is any, it will be assignable, and we'll get an error about the @ts-expect-error being unused.
I wish TypeScript had a way to compare types exactly, so I didn't have to do this.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, ok. I suspected it might be a negative assertion. Thanks for explaining and for adding this protection in.
|
|
||
| expect(parse(input, unionParser)).toEqual( | ||
| it("fails appropriately given an object with an invalid discriminant", () => { | ||
| expect(parse({shape: "squarle"}, parseUnion)).toEqual( |
There was a problem hiding this comment.
Ah, my favourite geometric shape, the squarle! 😂 🐿️
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Patch Changes - [#2064](#2064) [`55b4615d3`](55b4615) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-figures-aria flag - [#2063](#2063) [`85a5b5e44`](85a5b5e) Thanks [@nishasy](https://github.com/nishasy)! - Remove the interactive-graph-locked-features-labels flag - [#2078](#2078) [`781cc7df6`](781cc7d) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Temporarily fixing pi-based strings for Numeric Input - [#2065](#2065) [`eefcf5c5c`](eefcf5c) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-[figureName]-labels flags - [#2068](#2068) [`265a93104`](265a931) Thanks [@benchristel](https://github.com/benchristel)! - Internal: Redesign discriminated union type parser to have a simpler and more intuitive interface. - [#2073](#2073) [`4bf4960d4`](4bf4960) Thanks [@benchristel](https://github.com/benchristel)! - Internal: improve Perseus JSON parsers so they can handle all English-language exercises - [#2080](#2080) [`c9a28b34c`](c9a28b3) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Dropdown] Bugfix - Text in dropdown not in correct vertical position - [#2073](#2073) [`4bf4960d4`](4bf4960) Thanks [@benchristel](https://github.com/benchristel)! - Internal: improve the error messages produced by the versionedWidgetOptions parser ## @khanacademy/[email protected] ### Patch Changes - [#2064](#2064) [`55b4615d3`](55b4615) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-figures-aria flag - [#2063](#2063) [`85a5b5e44`](85a5b5e) Thanks [@nishasy](https://github.com/nishasy)! - Remove the interactive-graph-locked-features-labels flag - [#2065](#2065) [`eefcf5c5c`](eefcf5c) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-[figureName]-labels flags - Updated dependencies \[[`55b4615d3`](55b4615), [`85a5b5e44`](85a5b5e), [`781cc7df6`](781cc7d), [`eefcf5c5c`](eefcf5c), [`265a93104`](265a931), [`4bf4960d4`](4bf4960), [`c9a28b34c`](c9a28b3), [`4bf4960d4`](4bf4960)]: - @khanacademy/[email protected]
Previously, the
discriminatedUnionparser required you to specify thediscriminant via an object parser, which was confusing and verbose. Now you only
have to specify the discriminant key once, and provide the values for each
branch.
Issue: LEMS-2582
Test plan:
yarn test