From 9021862a236c89c205644e7ef32c2bdbce7c734d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Tue, 14 Mar 2023 16:50:25 +0100 Subject: [PATCH 1/7] tests salvaged from https://github.com/observablehq/plot/pull/1024 --- test/types/valueof-test.js | 46 +++++++++++++++++++++++++++++++++ test/types/valueof-type-test.ts | 29 +++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/types/valueof-test.js create mode 100644 test/types/valueof-type-test.ts diff --git a/test/types/valueof-test.js b/test/types/valueof-test.js new file mode 100644 index 0000000000..bfb9f9bf2f --- /dev/null +++ b/test/types/valueof-test.js @@ -0,0 +1,46 @@ +import assert from "assert"; +import {valueof} from "../../src/options.js"; + +it("valueof reads arrays", () => { + assert.deepStrictEqual( + valueof([1, 2, 3], (d) => d), + [1, 2, 3] + ); + assert.deepStrictEqual( + valueof([1, 2, 3], (d) => `${d}`, Array), + ["1", "2", "3"] + ); + assert.deepStrictEqual( + valueof([1, 2, 3], (d) => d, Float64Array), + Float64Array.of(1, 2, 3) + ); + assert.deepStrictEqual( + valueof([1, 2, 3], (d) => `${d}`, Float64Array), + Float64Array.of(1, 2, 3) + ); +}); + +// We don't expect the datum to be complicated things, but the user can provide +// their own, if they also provide the proper accessor function; here's a way to +// type this: +it("valueof can read arrays of complicated things", () => { + assert.deepStrictEqual( + valueof([(d) => d, new Promise(() => {})], (d) => `(${d})`), + ["((d) => d)", "([object Promise])"] + ); +}); + +it("data passed to valueof can be nullish and generated by the transform method", () => { + assert.deepStrictEqual(valueof(undefined, {transform: () => [1, "text"]}), [1, "text"]); + assert.deepStrictEqual(valueof(null, {transform: () => [1, "text"]}, Float32Array), Float32Array.of(1, NaN)); + assert.deepStrictEqual(valueof(null, {transform: () => new Float64Array(2)}, Array), [0, 0]); +}); + +// TODO? +it.skip("valueof does not crash on non iterable values with an accessor", () => { + for (const n of [null, undefined]) + assert.strictEqual( + valueof(n, () => 1), + n + ); +}); diff --git a/test/types/valueof-type-test.ts b/test/types/valueof-type-test.ts new file mode 100644 index 0000000000..d4e3fe16db --- /dev/null +++ b/test/types/valueof-type-test.ts @@ -0,0 +1,29 @@ +import {valueof} from "../../src/options.js"; + +// Valid input data +valueof([1, 2, 3], (d) => d, Float32Array); +valueof(["1", 2, 3], (d) => d); +valueof(["1", 2, 3], (d) => d, Array); +valueof(["1", 2, 3], (d) => d, Float64Array); +valueof(["1", 2, 3], (d) => +d, Float32Array); +valueof(new Set(["1", 2, 3]), (d) => +d, Float32Array); + +// A function is not a valid input data +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-expect-error +valueof(() => {}, "field"); + +// A Promise is not a valid input data +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-expect-error +valueof(new Promise(() => {}), "field"); + +// A symbol is not a valid value +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-expect-error +valueof(null, Symbol("field")); + +// A bigint is not a valid value +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-expect-error +valueof(null, 2n); From ae56d0c106e02ae93cd96444631465133336fa84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Tue, 14 Mar 2023 16:58:56 +0100 Subject: [PATCH 2/7] fix crash in valueof when the data is not iterable --- src/options.js | 2 +- test/types/valueof-test.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/options.js b/src/options.js index aa09100073..eafce6c1c5 100644 --- a/src/options.js +++ b/src/options.js @@ -128,7 +128,7 @@ export function arrayify(data) { // An optimization of type.from(values, f): if the given values are already an // instanceof the desired array type, the faster values.map method is used. export function map(values, f, type = Array) { - return values instanceof type ? values.map(f) : type.from(values, f); + return !values ? values : values instanceof type ? values.map(f) : type.from(values, f); } // An optimization of type.from(values): if the given values are already an diff --git a/test/types/valueof-test.js b/test/types/valueof-test.js index bfb9f9bf2f..6808b3907c 100644 --- a/test/types/valueof-test.js +++ b/test/types/valueof-test.js @@ -36,8 +36,7 @@ it("data passed to valueof can be nullish and generated by the transform method" assert.deepStrictEqual(valueof(null, {transform: () => new Float64Array(2)}, Array), [0, 0]); }); -// TODO? -it.skip("valueof does not crash on non iterable values with an accessor", () => { +it("valueof does not crash on non iterable values with an accessor", () => { for (const n of [null, undefined]) assert.strictEqual( valueof(n, () => 1), From e7c830bfb277b4a271927b4044ffa38aaa41b43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Tue, 14 Mar 2023 22:39:31 +0100 Subject: [PATCH 3/7] avoid the nullish test in map --- src/options.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/options.js b/src/options.js index eafce6c1c5..80e623f81f 100644 --- a/src/options.js +++ b/src/options.js @@ -7,6 +7,7 @@ export const TypedArray = Object.getPrototypeOf(Uint8Array); const objectToString = Object.prototype.toString; export function valueof(data, value, type) { + if (typeof value?.transform === "function") return maybeTypedArrayify(value.transform(data), type); const valueType = typeof value; return valueType === "string" ? maybeTypedMap(data, field(value), type) @@ -14,13 +15,11 @@ export function valueof(data, value, type) { ? maybeTypedMap(data, value, type) : valueType === "number" || value instanceof Date || valueType === "boolean" ? map(data, constant(value), type) - : value && typeof value.transform === "function" - ? maybeTypedArrayify(value.transform(data), type) : maybeTypedArrayify(value, type); } function maybeTypedMap(data, f, type) { - return map(data, type?.prototype instanceof TypedArray ? floater(f) : f, type); + return data == null ? data : map(data, type?.prototype instanceof TypedArray ? floater(f) : f, type); } function maybeTypedArrayify(data, type) { @@ -128,7 +127,7 @@ export function arrayify(data) { // An optimization of type.from(values, f): if the given values are already an // instanceof the desired array type, the faster values.map method is used. export function map(values, f, type = Array) { - return !values ? values : values instanceof type ? values.map(f) : type.from(values, f); + return values instanceof type ? values.map(f) : type.from(values, f); } // An optimization of type.from(values): if the given values are already an From e36922e507040b7f069ce1b8d0d891c9ee003163 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Tue, 14 Mar 2023 20:29:49 -0700 Subject: [PATCH 4/7] move null test back to map --- src/options.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/options.js b/src/options.js index 80e623f81f..1bd8b88b9e 100644 --- a/src/options.js +++ b/src/options.js @@ -7,7 +7,6 @@ export const TypedArray = Object.getPrototypeOf(Uint8Array); const objectToString = Object.prototype.toString; export function valueof(data, value, type) { - if (typeof value?.transform === "function") return maybeTypedArrayify(value.transform(data), type); const valueType = typeof value; return valueType === "string" ? maybeTypedMap(data, field(value), type) @@ -15,11 +14,13 @@ export function valueof(data, value, type) { ? maybeTypedMap(data, value, type) : valueType === "number" || value instanceof Date || valueType === "boolean" ? map(data, constant(value), type) + : typeof value?.transform === "function" + ? maybeTypedArrayify(value.transform(data), type) : maybeTypedArrayify(value, type); } function maybeTypedMap(data, f, type) { - return data == null ? data : map(data, type?.prototype instanceof TypedArray ? floater(f) : f, type); + return map(data, type?.prototype instanceof TypedArray ? floater(f) : f, type); } function maybeTypedArrayify(data, type) { @@ -127,7 +128,7 @@ export function arrayify(data) { // An optimization of type.from(values, f): if the given values are already an // instanceof the desired array type, the faster values.map method is used. export function map(values, f, type = Array) { - return values instanceof type ? values.map(f) : type.from(values, f); + return values == null ? values : values instanceof type ? values.map(f) : type.from(values, f); } // An optimization of type.from(values): if the given values are already an From 7f927a74cb8e7ec8e5fb807b94ed2a4b83b83ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Wed, 15 Mar 2023 08:53:18 +0100 Subject: [PATCH 5/7] consolidate tests --- ...valueof-type-test.ts => options-d-test.ts} | 2 +- test/options-test.js | 18 ++++++++ test/types/valueof-test.js | 45 ------------------- 3 files changed, 19 insertions(+), 46 deletions(-) rename test/{types/valueof-type-test.ts => options-d-test.ts} (95%) delete mode 100644 test/types/valueof-test.js diff --git a/test/types/valueof-type-test.ts b/test/options-d-test.ts similarity index 95% rename from test/types/valueof-type-test.ts rename to test/options-d-test.ts index d4e3fe16db..d0e3f0e06e 100644 --- a/test/types/valueof-type-test.ts +++ b/test/options-d-test.ts @@ -1,4 +1,4 @@ -import {valueof} from "../../src/options.js"; +import {valueof} from "../src/options.js"; // Valid input data valueof([1, 2, 3], (d) => d, Float32Array); diff --git a/test/options-test.js b/test/options-test.js index 4778494298..cda8b7ed72 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -114,3 +114,21 @@ it("valueof returns arrays that match the specified type as-is", () => { assert.strictEqual(valueof(m, identity, Float32Array), m); assert.strictEqual(valueof(m, identity, My32Array), m); }); + +it("valueof accepts complicated data with the proper accessor", () => { + const m = [(d) => d, new Promise(() => {})]; + assert.deepStrictEqual(valueof(m, String), ["(d) => d", "[object Promise]"]); +}); + +it("data passed to valueof can be nullish and generated by the transform method", () => { + assert.deepStrictEqual(valueof(undefined, {transform: () => [1, "text"]}), [1, "text"]); + assert.deepStrictEqual(valueof(null, {transform: () => [1, "text"]}, Float32Array), Float32Array.of(1, NaN)); + assert.deepStrictEqual(valueof(null, {transform: () => new Float64Array(2)}, Array), [0, 0]); +}); + +it("valueof does not crash on nullish data with an accessor", () => { + const a = () => 1; + assert.strictEqual(valueof(null, a), null); + assert.strictEqual(valueof(undefined, a), undefined); + assert.deepStrictEqual(valueof(0, a), []); // ill-defined, but not crashing +}); diff --git a/test/types/valueof-test.js b/test/types/valueof-test.js deleted file mode 100644 index 6808b3907c..0000000000 --- a/test/types/valueof-test.js +++ /dev/null @@ -1,45 +0,0 @@ -import assert from "assert"; -import {valueof} from "../../src/options.js"; - -it("valueof reads arrays", () => { - assert.deepStrictEqual( - valueof([1, 2, 3], (d) => d), - [1, 2, 3] - ); - assert.deepStrictEqual( - valueof([1, 2, 3], (d) => `${d}`, Array), - ["1", "2", "3"] - ); - assert.deepStrictEqual( - valueof([1, 2, 3], (d) => d, Float64Array), - Float64Array.of(1, 2, 3) - ); - assert.deepStrictEqual( - valueof([1, 2, 3], (d) => `${d}`, Float64Array), - Float64Array.of(1, 2, 3) - ); -}); - -// We don't expect the datum to be complicated things, but the user can provide -// their own, if they also provide the proper accessor function; here's a way to -// type this: -it("valueof can read arrays of complicated things", () => { - assert.deepStrictEqual( - valueof([(d) => d, new Promise(() => {})], (d) => `(${d})`), - ["((d) => d)", "([object Promise])"] - ); -}); - -it("data passed to valueof can be nullish and generated by the transform method", () => { - assert.deepStrictEqual(valueof(undefined, {transform: () => [1, "text"]}), [1, "text"]); - assert.deepStrictEqual(valueof(null, {transform: () => [1, "text"]}, Float32Array), Float32Array.of(1, NaN)); - assert.deepStrictEqual(valueof(null, {transform: () => new Float64Array(2)}, Array), [0, 0]); -}); - -it("valueof does not crash on non iterable values with an accessor", () => { - for (const n of [null, undefined]) - assert.strictEqual( - valueof(n, () => 1), - n - ); -}); From fbdaa5ee06c46177df4a6601553bf861f6ac23cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Wed, 15 Mar 2023 08:59:30 +0100 Subject: [PATCH 6/7] remove "valid input data" type tests --- test/options-d-test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/options-d-test.ts b/test/options-d-test.ts index d0e3f0e06e..1c905da9b2 100644 --- a/test/options-d-test.ts +++ b/test/options-d-test.ts @@ -1,13 +1,5 @@ import {valueof} from "../src/options.js"; -// Valid input data -valueof([1, 2, 3], (d) => d, Float32Array); -valueof(["1", 2, 3], (d) => d); -valueof(["1", 2, 3], (d) => d, Array); -valueof(["1", 2, 3], (d) => d, Float64Array); -valueof(["1", 2, 3], (d) => +d, Float32Array); -valueof(new Set(["1", 2, 3]), (d) => +d, Float32Array); - // A function is not a valid input data // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-expect-error From 6f07dc6ae9903affc1c419f5e6307b252996e800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Wed, 15 Mar 2023 09:30:47 +0100 Subject: [PATCH 7/7] test array value --- test/options-test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/options-test.js b/test/options-test.js index cda8b7ed72..4b96b06372 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -115,6 +115,13 @@ it("valueof returns arrays that match the specified type as-is", () => { assert.strictEqual(valueof(m, identity, My32Array), m); }); +it("valueof returns the given array value", () => { + const a = [1, 2, 3]; + assert.strictEqual(valueof(null, a), a); + assert.strictEqual(valueof(undefined, a), a); + assert.strictEqual(valueof([], a), a); +}); + it("valueof accepts complicated data with the proper accessor", () => { const m = [(d) => d, new Promise(() => {})]; assert.deepStrictEqual(valueof(m, String), ["(d) => d", "[object Promise]"]);