From fdf012bb2f2b4b57b2c9ff24a0cc859533df1f7e Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sun, 22 Oct 2023 16:53:28 +0200 Subject: [PATCH] Remove temporary behavior in fieldsAuto (#33) --- CHANGELOG.md | 45 ++++++++++++++++++++++++++++++++ README.md | 3 --- examples/extra-fields.test.ts | 49 ++++++++++++++++------------------- index.ts | 7 ----- tests/decoders.test.ts | 15 +++-------- 5 files changed, 72 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53432f4..76d798a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,50 @@ Note: I’m currently working on several breaking changes to tiny-decoders, but I’m trying out releasing them piece by piece. The idea is that you can either upgrade version by version only having to deal with one or a few breaking changes at a time, or wait and do a bunch of them at the same time. +### Version 13.0.0 (unreleased) + +> **Warning** +> This release contains a breaking change, but no TypeScript errors! Be careful! + +Version 11.0.0 made changes to `fieldsAuto`, but had a temporary behavior for backwards compatibility, awaiting the changes to `fieldsUnion` in version 12.0.0. This release (13.0.0) removes that temporary behavior. + +You need to be on the lookout for these two patterns: + +```ts +fieldsAuto({ + field1: undefinedOr(someDecoder), + field2: () => someValue, +}); +``` + +Previously, the above decoder would succeed even if `field1` or `field2` were missing. + +- If `field1` was missing, the temporary behavior in `fieldsAuto` would call the decoder at `field1` with `undefined`, which would succeed due to `undefinedOr`. If you did the version 11.0.0 migration perfectly, this shouldn’t matter. But upgrading to 13.0.0 might uncover some places where you use `undefinedOr(someDecoder)` but meant to use `field(someDecoder, { optional(true) })` or `field(undefinedOr(someDecoder), { optional(true) })` (the latter is the “safest” approach in that it is the most permissive). +- If `field2` was missing, the temporary behavior in `fieldsAuto` would call the decoder at `field2` with `undefined`, which would succeed due to that decoder ignoring its input and always succeeding with the same value. + +Here’s an example of how to upgrade the “always succeed” pattern: + +```ts +const productDecoder: Decoder = fieldsAuto({ + name: string, + price: number, + version: () => 1, +}); +``` + +Use `chain` instead: + +```ts +const productDecoder: Decoder = chain( + fieldsAuto({ + name: string, + price: number, + }), + (props) => ({ ...props, version: 1 }), +); +``` + +It’s a little bit more verbose, but unlocks further changes that will come in future releases. + ### Version 12.0.0 (2023-10-22) This release changes how `fieldsUnion` works. The new way should be easier to use, and it looks more similar to the type definition of a tagged union. diff --git a/README.md b/README.md index 0e9cd1d..0ee0c35 100644 --- a/README.md +++ b/README.md @@ -492,9 +492,6 @@ The `exact` option let’s you choose between ignoring extraneous data and makin See also the [Extra fields](examples/extra-fields.test.ts) example. -> **Warning** -> Temporary behavior: If a field is missing and _not_ marked as optional, `fieldsAuto` still _tries_ the decoder at the field (passing `undefined` to it). If the decoder succeeds (because it allows `undefined` or succeeds for any input), that value is used. If it fails, the regular “missing field” error is thrown. This means that `fieldsAuto({ name: undefinedOr(string) })` successfully produces `{ name: undefined }` if given `{}` as input. It is supposed to fail in that case (because a required field is missing), but temporarily it does not fail. This is to support how a previous version of `fieldsUnion` was used. Now `fieldsUnion` has been updated to a new API, so this temporary behavior in `fieldsAuto` will be removed in an upcoming version of tiny-decoders. - ### field ```ts diff --git a/examples/extra-fields.test.ts b/examples/extra-fields.test.ts index 7070061..c218940 100644 --- a/examples/extra-fields.test.ts +++ b/examples/extra-fields.test.ts @@ -13,13 +13,14 @@ test("adding extra fields to records", () => { const data: unknown = { name: "Comfortable Bed", price: 10e3 }; - // One way is to add a decoder that always succeeds - // (a function that ignores its input and always returns the same value). - const productDecoder: Decoder = fieldsAuto({ - name: string, - price: number, - version: () => 1, - }); + // Use `chain` to add it: + const productDecoder: Decoder = chain( + fieldsAuto({ + name: string, + price: number, + }), + (props) => ({ ...props, version: 1 }), + ); expect(productDecoder(data)).toMatchInlineSnapshot(` { @@ -29,28 +30,24 @@ test("adding extra fields to records", () => { } `); - // If you like, you can define this helper function: - const always = - (value: T) => - (): T => - value; - - const productDecoder2: Decoder = fieldsAuto({ + // In previous versions of tiny-decoders, another way of doing this was to add + // a decoder that always succeeds (a function that ignores its input and + // always returns the same value). + const productDecoderBroken: Decoder = fieldsAuto({ name: string, price: number, - version: always(1), + version: () => 1, }); - expect(productDecoder2(data)).toEqual(productDecoder(data)); - - // Finally, you can do it with `chain`. - const productDecoder3: Decoder = chain( - fieldsAuto({ - name: string, - price: number, - }), - (props) => ({ ...props, version: 1 }), - ); + // It no longer works, because all the fields you mentioned are expected to exist. + expect(() => productDecoderBroken(data)).toThrowErrorMatchingInlineSnapshot(` + "Expected an object with a field called: \\"version\\" + Got: { + \\"name\\": string, + \\"price\\": number + } + (Actual values are hidden in sensitive mode.) - expect(productDecoder3(data)).toEqual(productDecoder2(data)); + For better error messages, see https://github.com/lydell/tiny-decoders#error-messages" + `); }); diff --git a/index.ts b/index.ts index e53b819..8a1f612 100644 --- a/index.ts +++ b/index.ts @@ -221,13 +221,6 @@ export function fieldsAuto( knownFields.add(encodedFieldName); if (!(encodedFieldName in object)) { if (!isOptional) { - // TODO: Remove this try-catch when upgrading `fieldsUnion`. - try { - result[key] = decoder(undefined); - continue; - } catch { - // Prefer the below error. - } throw new DecoderError({ tag: "missing field", field: encodedFieldName, diff --git a/tests/decoders.test.ts b/tests/decoders.test.ts index edc17a6..9ab136f 100644 --- a/tests/decoders.test.ts +++ b/tests/decoders.test.ts @@ -1372,18 +1372,11 @@ describe("undefinedOr", () => { true, ); - // TODO: This is supposed to be an error. It will be once the temporary behavior in `fieldsAuto` is removed. - // expect(run(personDecoder, { name: "John" })).toMatchInlineSnapshot(` - // At root: - // Expected an object with a field called: "age" - // Got: { - // "name": "John" - // } - // `); expect(run(personDecoder, { name: "John" })).toMatchInlineSnapshot(` - { - "age": undefined, - "name": "John", + At root: + Expected an object with a field called: "age" + Got: { + "name": "John" } `);