resource component refactor, adjust integration tests

QwikDev · Jun 22, 2024 · 462f393 · 462f393
1 parent 40ae750
commit 462f393
Show file tree

Hide file tree

Showing 4 changed files with 179 additions and 63 deletions.
diff --git a/packages/qwik/src/core/use/use-resource.ts b/packages/qwik/src/core/use/use-resource.ts
@@ -23,6 +23,13 @@ import { isObject } from '../util/types';
 import { useSequentialScope } from './use-sequential-scope';
 import type { fixMeAny } from '../../server/qwik-types';
 
+const DEBUG: boolean = false;
+
+function debugLog(...arg: any) {
+  // eslint-disable-next-line no-console
+  console.log(arg.join(', '));
+}
+
 /**
  * Options to pass to `useResource$()`
  *
@@ -248,50 +255,48 @@ export interface ResourceProps<T> {
  */
 // </docs>
 export const Resource = <T>(props: ResourceProps<T>): JSXOutput => {
-  const isBrowser = !isServerPlatform();
+  // Resource path
+  return _jsxSorted(Fragment, null, null, getResourceValueAsPromise(props), 0, null);
+};
+
+function getResourceValueAsPromise<T>(props: ResourceProps<T>): Promise<JSXOutput> | JSXOutput {
   const resource = props.value as ResourceReturnInternal<T> | Promise<T> | Signal<T>;
-  let promise: Promise<T> | undefined;
   if (isResourceReturn(resource)) {
+    const isBrowser = !isServerPlatform();
     if (isBrowser) {
-      if (props.onRejected) {
-        resource.value.catch(() => {});
-        if (resource._state === 'rejected') {
-          return props.onRejected(resource._error!);
-        }
-      }
-      if (props.onPending) {
-        const state = resource._state;
-        if (state === 'resolved') {
-          return props.onResolved(resource._resolved!);
-        } else if (state === 'pending') {
-          return props.onPending();
-        } else if (state === 'rejected') {
-          throw resource._error;
-        }
-      }
-      if (untrack(() => resource._resolved) !== undefined) {
-        return props.onResolved(resource._resolved!);
+      const state = resource._state;
+      DEBUG && debugLog(`RESOURCE_CMP.${state}`, 'VALUE: ' + untrack(() => resource._resolved));
+
+      if (state === 'pending' && props.onPending) {
+        return Promise.resolve(props.onPending());
+      } else if (state === 'rejected' && props.onRejected) {
+        return Promise.resolve(resource._error!).then(props.onRejected);
+      } else {
+        // resolved, pending without onPending prop or rejected with onRejected prop
+        return Promise.resolve(resource._resolved as T).then(props.onResolved);
       }
     }
-    promise = resource.value;
+    return resource.value.then(
+      useBindInvokeContext(props.onResolved),
+      useBindInvokeContext(props.onRejected)
+    );
   } else if (isPromise(resource)) {
-    promise = resource;
+    return resource.then(
+      useBindInvokeContext(props.onResolved),
+      useBindInvokeContext(props.onRejected)
+    );
   } else if (isSignal(resource)) {
-    promise = Promise.resolve(resource.value);
+    return Promise.resolve(resource.value).then(
+      useBindInvokeContext(props.onResolved),
+      useBindInvokeContext(props.onRejected)
+    );
   } else {
-    return props.onResolved(resource as T);
+    return Promise.resolve(resource as T).then(
+      useBindInvokeContext(props.onResolved),
+      useBindInvokeContext(props.onRejected)
+    );
   }
-
-  // Resource path
-  return _jsxSorted(
-    Fragment,
-    null,
-    null,
-    promise.then(useBindInvokeContext(props.onResolved), useBindInvokeContext(props.onRejected)),
-    0,
-    null
-  );
-};
+}
 
 export const _createResourceReturn = <T>(opts?: ResourceOptions): ResourceReturnInternal<T> => {
   const resource: ResourceReturnInternal<T> = {
@@ -326,6 +331,7 @@ export const isResourceReturn = (obj: any): obj is ResourceReturn<unknown> => {
   return isObject(obj) && (getProxyTarget(obj as any) || obj).__brand === 'resource';
 };
 
+// TODO: to remove - serializers v1
 export const serializeResource = (
   resource: ResourceReturnInternal<unknown>,
   getObjId: GetObjID
@@ -340,9 +346,10 @@ export const serializeResource = (
   }
 };
 
+// TODO: to remove - serializers v1
 export const parseResourceReturn = <T>(data: string): ResourceReturnInternal<T> => {
   const [first, id] = data.split(' ');
-  const result = _createResourceReturn<T>(undefined);
+  const result = _createResourceReturn<T>();
   result.value = Promise.resolve() as any;
   if (first === '0') {
     result._state = 'resolved';

diff --git a/packages/qwik/src/core/use/use-task.ts b/packages/qwik/src/core/use/use-task.ts
@@ -697,7 +697,7 @@ export const runResource = <T>(
   const resource = task.$state$;
   assertDefined(
     resource,
-    'useResource: when running a resource, "task.r" must be a defined.',
+    'useResource: when running a resource, "task.resource" must be a defined.',
     task
   );
 
@@ -773,6 +773,7 @@ export const runResource = <T>(
         resource._state = 'resolved';
         resource._resolved = value as T;
         resource._error = undefined;
+        // console.log('RESOURCE.resolved: ', value);
 
         resolve(value as T);
       } else {
@@ -789,6 +790,7 @@ export const runResource = <T>(
 
   // Execute mutation inside empty invocation
   invoke(iCtx, () => {
+    // console.log('RESOURCE.pending: ');
     resource._state = 'pending';
     resource.loading = !isServerPlatform();
     const promise = (resource.value = new Promise((r, re) => {
@@ -799,7 +801,7 @@ export const runResource = <T>(
   });
 
   const promise = safeCall(
-    () => Promise.resolve(taskFn(opts)),
+    () => taskFn(opts),
     (value) => {
       setState(true, value);
     },

diff --git a/packages/qwik/src/core/v2/shared/scheduler.ts b/packages/qwik/src/core/v2/shared/scheduler.ts
@@ -96,6 +96,7 @@ import {
 } from '../../use/use-task';
 import { isPromise, maybeThen, maybeThenPassError, safeCall } from '../../util/promises';
 import type { ValueOrPromise } from '../../util/types';
+import { isDomContainer } from '../client/dom-container';
 import type { VirtualVNode } from '../client/types';
 import { vnode_documentPosition, vnode_isVNode } from '../client/vnode';
 import { vnode_diff } from '../client/vnode-diff';
@@ -292,11 +293,12 @@ export const createScheduler = (
         returnValue = runComputed2(chore.$payload$ as Task<TaskFn, TaskFn>, container, host);
         break;
       case ChoreType.RESOURCE:
-        // Ignore the return value of the resource, because async resources should not be awaited.
+        // Don't await the return value of the resource, because async resources should not be awaited.
         // The reason for this is that we should be able to update for example a node with loading
         // text. If we await the resource, the loading text will not be displayed until the resource
         // is loaded.
-        runResource(chore.$payload$ as ResourceDescriptor<TaskFn>, container, host);
+        const result = runResource(chore.$payload$ as ResourceDescriptor<TaskFn>, container, host);
+        returnValue = isDomContainer(container) ? null : result;
         break;
       case ChoreType.TASK:
       case ChoreType.VISIBLE: