From 32aa744f1dad9e7daab0661acc62baec27d7b40d Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Fri, 1 Nov 2024 17:01:46 +0100 Subject: [PATCH] Improve resolving arguments in createResource (#2353) * Improve resolving arguments in createResource Check typeof the second argument instead of using the `arguments` variable. This fixes usedase where some arguments were passed explicitely as `undefined` instead of being omitted. * Create twelve-comics-knock.md --------- Co-authored-by: Ryan Carniato --- .changeset/twelve-comics-knock.md | 5 ++++ packages/solid/src/reactive/signal.ts | 11 +++++---- packages/solid/src/server/rendering.ts | 13 ++++------- packages/solid/test/resource.spec.ts | 32 ++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 .changeset/twelve-comics-knock.md diff --git a/.changeset/twelve-comics-knock.md b/.changeset/twelve-comics-knock.md new file mode 100644 index 000000000..bfbf6cfd8 --- /dev/null +++ b/.changeset/twelve-comics-knock.md @@ -0,0 +1,5 @@ +--- +"solid-js": patch +--- + +Improve resolving arguments in createResource diff --git a/packages/solid/src/reactive/signal.ts b/packages/solid/src/reactive/signal.ts index ab86cf44b..fd1ab94d2 100644 --- a/packages/solid/src/reactive/signal.ts +++ b/packages/solid/src/reactive/signal.ts @@ -591,14 +591,15 @@ export function createResource( let source: ResourceSource; let fetcher: ResourceFetcher; let options: ResourceOptions; - if ((arguments.length === 2 && typeof pFetcher === "object") || arguments.length === 1) { - source = true as ResourceSource; - fetcher = pSource as ResourceFetcher; - options = (pFetcher || {}) as ResourceOptions; - } else { + + if (typeof pFetcher === "function") { source = pSource as ResourceSource; fetcher = pFetcher as ResourceFetcher; options = pOptions || ({} as ResourceOptions); + } else { + source = true as ResourceSource; + fetcher = pSource as ResourceFetcher; + options = (pFetcher || {}) as ResourceOptions; } let pr: Promise | null = null, diff --git a/packages/solid/src/server/rendering.ts b/packages/solid/src/server/rendering.ts index 425e77350..e14cdf42e 100644 --- a/packages/solid/src/server/rendering.ts +++ b/packages/solid/src/server/rendering.ts @@ -374,16 +374,13 @@ export function createResource( fetcher?: ResourceFetcher | ResourceOptions | ResourceOptions, options: ResourceOptions | ResourceOptions = {} ): ResourceReturn | ResourceReturn { - if (arguments.length === 2) { - if (typeof fetcher === "object") { - options = fetcher as ResourceOptions | ResourceOptions; - fetcher = source as ResourceFetcher; - source = true as ResourceSource; - } - } else if (arguments.length === 1) { - fetcher = source as ResourceFetcher; + + if (typeof fetcher !== "function") { source = true as ResourceSource; + fetcher = source as ResourceFetcher; + options = (fetcher || {}) as ResourceOptions | ResourceOptions; } + const contexts = new Set(); const id = sharedConfig.getNextContextId(); let resource: { ref?: any; data?: T } = {}; diff --git a/packages/solid/test/resource.spec.ts b/packages/solid/test/resource.spec.ts index 9874e6b2b..71f9f8d3d 100644 --- a/packages/solid/test/resource.spec.ts +++ b/packages/solid/test/resource.spec.ts @@ -385,3 +385,35 @@ describe("using Resource with custom store", () => { expect(street).toBe(2); }); }); + +describe("createResource can be wrapped", () => { + + const fns: [name: string, function: typeof createResource][] = [ + ["original createResource", createResource], + // @ts-ignore + ["createResource(...args)", (...args) => createResource(...args)], + // @ts-ignore + ["createResource(a, b, c)", (a, b, c) => createResource(a, b, c)], + ] + + for (const [name, fn] of fns) { + + test(`only fetcher in ${name}`, () => { + const [[data], dispose] = createRoot(dispose => [fn(() => 123), dispose]) + expect(data()).toBe(123) + dispose() + }) + + test(`fetcher and source in ${name}`, () => { + const [source, setSource] = createSignal(1) + + const [[data], dispose] = createRoot(dispose => [fn(source, v => v), dispose]) + expect(data()).toBe(1) + + setSource(2) + expect(data()).toBe(2) + + dispose() + }) + } +})