From e944f4827cec419f8806a56552c6272047b4fd72 Mon Sep 17 00:00:00 2001 From: Shinrai Date: Sat, 3 Oct 2026 19:38:21 -0700 Subject: [PATCH 1/2] fix: revive a namespace without a default export, and report a non-JSON validate rejection Both bugs live in the two import() strategy blocks of wisp(), so they are fixed together. A reviver or validate on a module with no default export passed the module namespace to structuredClone, which cannot clone a namespace object. The DataCloneError was swallowed as a failed strategy and the load ended in the generic "Unsupported type" error. wisp now copies the namespace's own enumerable exports to a plain object first, consistent with returning the namespace itself when no options are given; a reviver gets that copy through JSON.parse, which already returns a fresh value. wispSync only parses JSON and never sees a namespace, so it is unaffected. validate ran inside each import() strategy's try block, so a rejection counted as a strategy failure: the next strategy re-imported the module and validated again, and a non-JSON type ended in "Unsupported type". validate now runs after the strategy loop on every path and throws the same error as the fs path, "Failed to load JSON file at : @cldmv/wisp: ", with the original error as the cause. A rejecting validate is now called once instead of once per strategy. Fixes #38 Fixes #39 --- README.md | 2 +- src/wisp.mjs | 63 +++++++++---------- tests/import-strategies.test.vitest.mjs | 82 ++++++++++++++++++------- types/wisp.d.mts.map | 2 +- 4 files changed, 91 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index a4218cc..950a0b7 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Asynchronously loads JSON from a file. - `options` (object, optional): - `base` (string | URL, optional): Base URL for resolving relative paths. Defaults to the caller's file URL. - `validate` (function, optional): Validation function called with the parsed JSON. Throws if validation fails. - - `reviver` (function, optional): Reviver function passed to `JSON.parse`. + - `reviver` (function, optional): Reviver function passed to `JSON.parse`. For a module loaded through `import()` that has no default export, `reviver` and `validate` receive a plain-object copy of its exports. - `type` (string, optional): Import attribute type used for the `import()` attempts. Defaults to `"json"`; the file-system fallback only runs for `"json"`. - `fallback` (string | URL, optional): A second file to load when `input` cannot be read or parsed. A `validate` failure on `input` throws rather than falling back. diff --git a/src/wisp.mjs b/src/wisp.mjs index ba3d790..a03ac76 100644 --- a/src/wisp.mjs +++ b/src/wisp.mjs @@ -41,21 +41,33 @@ import { resolveUrlFromCaller } from "./lib/resolve-from-caller.mjs"; * @param {*} v - The value to clone. * @returns {*} The cloned value. */ +function deepClone(v) { + return typeof globalThis.structuredClone === "function" ? globalThis.structuredClone(v) : JSON.parse(JSON.stringify(v)); +} + /** - * Deep clones a value using structuredClone if available, otherwise JSON.parse/stringify. + * Picks the value a loaded module provides: its default export, or the namespace when it has none. + * With a reviver or validate the value is copied so the module cache is never mutated; a namespace + * object cannot be structured-cloned, so it is first copied to a plain object of its exports. * @private - * @param {*} v - The value to clone. - * @returns {*} The cloned value. + * @param {Record} mod - The module namespace returned by import(). + * @param {((this: any, key: string, value: any) => any)|undefined} reviver - Reviver function, if any. + * @param {((val: any) => void)|undefined} validate - Validation function, if any. + * @returns {*} The module's value, copied when a reviver or validate is given. */ -function deepClone(v) { - return typeof globalThis.structuredClone === "function" ? globalThis.structuredClone(v) : JSON.parse(JSON.stringify(v)); +function moduleValue(mod, reviver, validate) { + const value = mod.default ?? mod; + if (!reviver && !validate) return value; + const data = value === mod ? { ...mod } : value; + // JSON.parse already returns a fresh value, so a reviver needs no separate clone. + return reviver ? JSON.parse(JSON.stringify(data), reviver) : deepClone(data); } /** * Runs the caller's validation on a loaded value, throwing the wisp-prefixed load error when it rejects the data. * @private * @param {((val: any) => void)|undefined} validate - Validation function, if any. - * @param {*} val - The parsed JSON value. + * @param {*} val - The loaded value. * @param {URL} url - The URL the value was loaded from. * @returns {void} */ @@ -100,36 +112,19 @@ export async function wisp(input, options = {}) { else url = new URL(resolveUrlFromCaller(s)); } - try { - const mod = await import(url.href, { with: { type } }); - if (!reviver && !validate) return mod?.default ?? mod; - let val = deepClone(mod?.default ?? mod); - if (reviver) val = JSON.parse(JSON.stringify(val), reviver); - if (validate) { - try { - validate(val); - } catch (e) { - throw new Error(`@cldmv/wisp: ${e?.message ?? e}`, { cause: e }); - } - } - return val; - } catch {} - - try { - // Legacy import assertions (`assert`) for Node 16.14-20.9, which predate `with`; the cast keeps the type checker from rejecting the key. - const mod = await import(url.href, /** @type {any} */ ({ assert: { type } })); - if (!reviver && !validate) return mod?.default ?? mod; - let val = deepClone(mod?.default ?? mod); - if (reviver) val = JSON.parse(JSON.stringify(val), reviver); - if (validate) { - try { - validate(val); - } catch (e) { - throw new Error(`@cldmv/wisp: ${e?.message ?? e}`, { cause: e }); - } + // Each import() strategy only has to load the module; validate runs after the strategy loop, so a + // rejection is reported as the validation error instead of being treated as a failed strategy. + // Legacy import assertions (`assert`) serve Node 16.14-20.9, which predate `with`; the cast keeps the type checker from rejecting the key. + for (const attributes of [{ with: { type } }, /** @type {any} */ ({ assert: { type } })]) { + let val; + try { + val = moduleValue(await import(url.href, attributes), reviver, validate); + } catch { + continue; } + runValidate(validate, val, url); return val; - } catch {} + } if (type === "json") { let val; diff --git a/tests/import-strategies.test.vitest.mjs b/tests/import-strategies.test.vitest.mjs index be0fb98..9612579 100644 --- a/tests/import-strategies.test.vitest.mjs +++ b/tests/import-strategies.test.vitest.mjs @@ -105,6 +105,19 @@ describe("import() with `with` attributes", () => { ); }); + it("runs a rejecting validate once instead of once per load strategy", async () => { + let calls = 0; + await expect( + wisp(sample, { + validate: () => { + calls++; + throw new Error("once"); + } + }) + ).rejects.toThrow(/: @cldmv\/wisp: once$/); + expect(calls).toBe(1); + }); + it("keeps the validation error as the cause", async () => { const original = new Error("nope"); const err = await wisp(sample, { @@ -169,37 +182,62 @@ describe.runIf(retriesFailedImport)("import() with legacy `assert` attributes", const first = await wisp(url, { type: "javascript" }); const second = await wisp(url, { type: "javascript" }); expect(second).toBe(first); - // A reviver needs a clone, which a namespace object cannot give, on every strategy. - await expect(wisp(url, { type: "javascript", reviver: (key, value) => value })).rejects.toThrow( - /^@cldmv\/wisp: Unsupported type 'javascript'/ - ); + // The cached namespace has no default export, so the reviver gets a plain-object copy of it. + const revived = await wisp(url, { type: "javascript", reviver: (key, value) => (key === "other" ? value + 1 : value) }); + expect(revived).toEqual({ named: "value", other: 3 }); + // A validation failure on the `with` attempt surfaces as the validation error too. + await expect( + wisp(url, { + type: "javascript", + validate: () => { + throw "rejected on with"; + } + }) + ).rejects.toThrow(/^@cldmv\/wisp: Failed to load JSON file at file:.*no-default\.mjs\?case=\d+: @cldmv\/wisp: rejected on with$/); }); - it("cannot clone a namespace without a default export, so a reviver ends in the unsupported-type error", async () => { - // structuredClone rejects a module namespace object; that failure is swallowed like any - // other strategy failure, and the fs.readFile strategy only handles type "json". - await expect(wisp(fresh(noDefaultFile), { type: "javascript", reviver: (key, value) => value })).rejects.toThrow( - /^@cldmv\/wisp: Unsupported type 'javascript' or failed to load module at file:.*no-default\.mjs\?case=\d+$/ - ); + it("applies a reviver to a plain-object copy of a namespace without a default export", async () => { + const keys = []; + const data = await wisp(fresh(noDefaultFile), { + type: "javascript", + reviver: (key, value) => { + keys.push(key); + return key === "named" ? "revived" : value; + } + }); + expect(data).toEqual({ named: "revived", other: 2 }); + expect(Object.getPrototypeOf(data)).toBe(Object.prototype); + expect(keys).toEqual(["named", "other", ""]); + }); + + it("validates a plain-object copy of a namespace without a default export", async () => { + const seen = []; + const data = await wisp(fresh(noDefaultFile), { type: "javascript", validate: (val) => seen.push(val) }); + expect(data).toEqual({ named: "value", other: 2 }); + expect(Object.getPrototypeOf(data)).toBe(Object.prototype); + expect(seen).toEqual([data]); }); - it("reports a validation failure on a non-JSON module as an unsupported type", async () => { - // The `assert` attempt's validation error is swallowed like any other failure, and the - // fs.readFile strategy only handles type "json", so the final error is the unsupported-type one. + it("reports a validation failure on a non-JSON module as the validation error", async () => { + let calls = 0; const validate = () => { + calls++; throw "rejected"; }; await expect(wisp(fresh(moduleFile), { type: "javascript", validate })).rejects.toThrow( - /^@cldmv\/wisp: Unsupported type 'javascript' or failed to load module at file:.*module\.mjs\?case=\d+$/ + /^@cldmv\/wisp: Failed to load JSON file at file:.*module\.mjs\?case=\d+: @cldmv\/wisp: rejected$/ ); - await expect( - wisp(fresh(moduleFile), { - type: "javascript", - validate: () => { - throw new Error("rejected as Error"); - } - }) - ).rejects.toThrow(/^@cldmv\/wisp: Unsupported type 'javascript'/); + // The rejection is final: no later strategy loads the module again and re-runs validate. + expect(calls).toBe(1); + const original = new Error("rejected as Error"); + const err = await wisp(fresh(moduleFile), { + type: "javascript", + validate: () => { + throw original; + } + }).catch((e) => e); + expect(err.message).toMatch(/: @cldmv\/wisp: rejected as Error$/); + expect(err.cause).toBe(original); }); }); diff --git a/types/wisp.d.mts.map b/types/wisp.d.mts.map index 7d6f57b..7ea6996 100644 --- a/types/wisp.d.mts.map +++ b/types/wisp.d.mts.map @@ -1 +1 @@ -{"version":3,"file":"wisp.d.mts","sourceRoot":"","sources":["../src/wisp.mjs"],"names":[],"mappings":"AAsEA;;;;;;;;;;;;;;;;;;;GAmBG;AACH,4BAjBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,OAAO,CAAC,GAAC,CAAC,CAuEtB;AAED;;;;;;;;;;;;;;;;;;GAkBG;AACH,gCAhBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,GAAC,CAmCb"} \ No newline at end of file +{"version":3,"file":"wisp.d.mts","sourceRoot":"","sources":["../src/wisp.mjs"],"names":[],"mappings":"AAkFA;;;;;;;;;;;;;;;;;;;GAmBG;AACH,4BAjBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,OAAO,CAAC,GAAC,CAAC,CAsDtB;AAED;;;;;;;;;;;;;;;;;;GAkBG;AACH,gCAhBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,GAAC,CAmCb"} \ No newline at end of file From c33cd39e373194fc3be72c8afbc3b87e385a25db Mon Sep 17 00:00:00 2001 From: Shinrai Date: Sat, 3 Oct 2026 20:03:37 -0700 Subject: [PATCH 2/2] refactor: pass the import attributes through a helper in the strategy loop CodeQL's js/unused-loop-variable flagged the for...of variable, which was only used as import()'s options argument. Loading through a small helper makes the use explicit; behaviour is unchanged. --- src/wisp.mjs | 3 ++- types/wisp.d.mts.map | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wisp.mjs b/src/wisp.mjs index a03ac76..06271c8 100644 --- a/src/wisp.mjs +++ b/src/wisp.mjs @@ -115,10 +115,11 @@ export async function wisp(input, options = {}) { // Each import() strategy only has to load the module; validate runs after the strategy loop, so a // rejection is reported as the validation error instead of being treated as a failed strategy. // Legacy import assertions (`assert`) serve Node 16.14-20.9, which predate `with`; the cast keeps the type checker from rejecting the key. + const loadWith = async (attributes) => moduleValue(await import(url.href, attributes), reviver, validate); for (const attributes of [{ with: { type } }, /** @type {any} */ ({ assert: { type } })]) { let val; try { - val = moduleValue(await import(url.href, attributes), reviver, validate); + val = await loadWith(attributes); } catch { continue; } diff --git a/types/wisp.d.mts.map b/types/wisp.d.mts.map index 7ea6996..4615c38 100644 --- a/types/wisp.d.mts.map +++ b/types/wisp.d.mts.map @@ -1 +1 @@ -{"version":3,"file":"wisp.d.mts","sourceRoot":"","sources":["../src/wisp.mjs"],"names":[],"mappings":"AAkFA;;;;;;;;;;;;;;;;;;;GAmBG;AACH,4BAjBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,OAAO,CAAC,GAAC,CAAC,CAsDtB;AAED;;;;;;;;;;;;;;;;;;GAkBG;AACH,gCAhBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,GAAC,CAmCb"} \ No newline at end of file +{"version":3,"file":"wisp.d.mts","sourceRoot":"","sources":["../src/wisp.mjs"],"names":[],"mappings":"AAkFA;;;;;;;;;;;;;;;;;;;GAmBG;AACH,4BAjBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,OAAO,CAAC,GAAC,CAAC,CAuDtB;AAED;;;;;;;;;;;;;;;;;;GAkBG;AACH,gCAhBW,MAAM,GAAC,GAAG,YAElB;IAA6B,IAAI,GAAzB,MAAM,GAAC,GAAG;IACmB,QAAQ,GAArC,CAAC,GAAG,EAAE,GAAG,KAAK,IAAI;IACoC,OAAO,GAA7D,CAAC,IAAI,EAAE,GAAG,EAAE,GAAG,EAAE,MAAM,EAAE,KAAK,EAAE,GAAG,KAAK,GAAG;IAC1B,IAAI,GAArB,MAAM;IACe,QAAQ,GAA7B,MAAM,GAAC,GAAG;CAClB,GAAU,GAAC,CAmCb"} \ No newline at end of file