fix: rendering state relative to q:container (#322)

Author: manucorporat

src/bootloader-shared.ts

@@ -24,29 +24,11 @@
  * @param href
  * @returns
  */
-export const qrlResolver = (
-  doc: Document,
-  element: Element | null,
-  eventUrl?: string | null,
-  _url?: string,
-  _base?: string | URL
-): URL | undefined => {
-  if (eventUrl === undefined) {
-    //  recursive call
-    if (element) {
-      _url = element.getAttribute('q:base')!;
-      _base = qrlResolver(
-        doc,
-        element.parentNode && (element.parentNode as HTMLElement).closest('[q\\:base]')
-      );
-    } else {
-      _url = doc.baseURI;
-    }
-  } else if (eventUrl) {
-    _url = eventUrl;
-    _base = qrlResolver(doc, element!.closest('[q\\:base]'));
-  }
-  return _url ? new URL(_url, _base) : undefined;
+export const qrlResolver = (element: Element, eventUrl: string): URL => {
+  const doc = element.ownerDocument!;
+  const containerEl = element.closest('[q\\:container]');
+  const base = new URL(containerEl?.getAttribute('q:base') ?? doc.baseURI, doc.baseURI);
+  return new URL(eventUrl, base);
 };
 
 const error = (msg: string) => {
@@ -92,7 +74,7 @@ export const qwikLoader = (doc: Document, hasInitialized?: boolean | number) =>
         ev.preventDefault();
       }
       for (const qrl of attrValue.split('\n')) {
-        const url = qrlResolver(doc, element, qrl);
+        const url = qrlResolver(element, qrl);
         if (url) {
           const symbolName = getSymbolName(url);
           const module =
@@ -223,7 +205,7 @@ export const setupPrefetching = (
           const name = attr.name;
           const value = attr.value;
           if (name.startsWith('on:') && value) {
-            const url = qrlResolver(doc, element, value)!;
+            const url = qrlResolver(element, value)!;
             url.hash = url.search = '';
             const key = url.toString() + '.js';
             if (!qrlCache[key]) {

src/bootloader-shared.unit.ts

@@ -100,36 +100,29 @@ describe('qwikloader', () => {
 
     it('should resolve full URL', () => {
       const div = doc.createElement('div');
-      expect(String(qrlResolver(doc, div, 'http://foo.bar/baz'))).toEqual('http://foo.bar/baz');
+      expect(String(qrlResolver(div, 'http://foo.bar/baz'))).toEqual('http://foo.bar/baz');
     });
 
     it('should resolve relative URL against base', () => {
       const div = doc.createElement('div');
-      expect(String(qrlResolver(doc, div, './bar'))).toEqual('http://document.qwik.dev/bar');
+      expect(String(qrlResolver(div, './bar'))).toEqual('http://document.qwik.dev/bar');
     });
 
     it('should resolve relative URL against q:base', () => {
       const div = doc.createElement('div');
-      div.setAttribute('q:base', '../baz/');
-      expect(String(qrlResolver(doc, div, './bar'))).toEqual('http://document.qwik.dev/baz/bar');
+      div.setAttribute('q:container', '');
+      div.setAttribute('q:base', '/baz/');
+      expect(String(qrlResolver(div, './bar'))).toEqual('http://document.qwik.dev/baz/bar');
     });
 
     it('should resolve relative URL against nested q:base', () => {
       const div = doc.createElement('div');
       const parent = doc.createElement('parent');
       doc.body.appendChild(parent);
       parent.appendChild(div);
+      parent.setAttribute('q:container', '');
       parent.setAttribute('q:base', './parent/');
-      div.setAttribute('q:base', './child/');
-      expect(String(qrlResolver(doc, div, './bar'))).toEqual(
-        'http://document.qwik.dev/parent/child/bar'
-      );
-    });
-
-    it('do nothing for null/undefined/empty string', () => {
-      const div = doc.createElement('div');
-      expect(qrlResolver(doc, null, null)).toBeFalsy();
-      expect(qrlResolver(doc, div, '')).toBeFalsy();
+      expect(String(qrlResolver(div, './bar'))).toEqual('http://document.qwik.dev/parent/bar');
     });
   });
 

src/core/component/component-ctx.ts

@@ -7,6 +7,12 @@ import { styleContent, styleHost } from './qrl-styles';
 import { newInvokeContext, useInvoke } from '../use/use-core';
 import type { QContext } from '../props/props';
 import { processNode } from '../render/jsx/jsx-runtime';
+import type { QRLInternal } from '../import/qrl-class';
+
+export interface RenderFactoryOutput {
+  renderQRL: QRLInternal;
+  waitOn: any[];
+}
 
 export const firstRenderComponent = (rctx: RenderContext, ctx: QContext) => {
   ctx.element.setAttribute(QHostAttr, '');

src/core/component/component.public.ts

@@ -1,9 +1,9 @@
 import { toQrlOrError } from '../import/qrl';
 import type { QRLInternal } from '../import/qrl-class';
 import { $, implicit$FirstArg, QRL } from '../import/qrl.public';
-import { qPropWriteQRL, qrlFactory } from '../props/props-on';
+import { qPropWriteQRL } from '../props/props-on';
 import type { JSXNode } from '../render/jsx/types/jsx-node';
-import { newInvokeContext, useInvoke, useWaitOn } from '../use/use-core';
+import { newInvokeContext, StyleAppend, useInvoke, useWaitOn } from '../use/use-core';
 import { useHostElement } from '../use/use-host-element.public';
 import { ComponentScopedStyles, OnRenderProp } from '../util/markers';
 import { styleKey } from './qrl-styles';
@@ -14,6 +14,8 @@ import type { FunctionComponent } from '../index';
 import { jsx } from '../render/jsx/jsx-runtime';
 
 import { getDocument } from '../util/dom';
+import { promiseAll } from '../util/promises';
+import type { RenderFactoryOutput } from './component-ctx';
 
 // <docs markdown="https://hackmd.io/c_nNpiLZSYugTU0c5JATJA#onUnmount">
 // !!DO NOT EDIT THIS COMMENT DIRECTLY!!!
@@ -323,14 +325,17 @@ export function componentQrl<PROPS extends {}>(
 
   // Return a QComponent Factory function.
   return function QComponent(props, key): JSXNode<PROPS> {
-    const onRenderFactory: qrlFactory = async (hostElement: Element): Promise<QRLInternal> => {
-      // Turn function into QRL
+    const onRenderFactory = async (hostElement: Element): Promise<RenderFactoryOutput> => {
       const onMountQrl = toQrlOrError(onMount);
       const onMountFn = await resolveQrl(hostElement, onMountQrl);
       const ctx = getContext(hostElement);
       const props = getProps(ctx) as any;
       const invokeCtx = newInvokeContext(getDocument(hostElement), hostElement, hostElement);
-      return useInvoke(invokeCtx, onMountFn, props) as QRLInternal;
+      const renderQRL = (await useInvoke(invokeCtx, onMountFn, props)) as QRLInternal;
+      return {
+        renderQRL,
+        waitOn: await promiseAll(invokeCtx.waitOn || []),
+      };
     };
     onRenderFactory.__brand__ = 'QRLFactory';
 
@@ -433,14 +438,12 @@ function _useStyles(styles: QRL<string>, scoped: boolean) {
 
   useWaitOn(
     styleQrl.resolve(hostElement).then((styleText) => {
-      const document = getDocument(hostElement);
-      const head = document.querySelector('head');
-      if (head && !head.querySelector(`style[q\\:style="${styleId}"]`)) {
-        const style = document.createElement('style');
-        style.setAttribute('q:style', styleId);
-        style.textContent = scoped ? styleText.replace(/�/g, styleId) : styleText;
-        head.appendChild(style);
-      }
+      const task: StyleAppend = {
+        type: 'style',
+        scope: styleId,
+        content: scoped ? styleText.replace(/�/g, styleId) : styleText,
+      };
+      return task;
     })
   );
 }

src/core/component/mock.unit.css

@@ -1,12 +1,12 @@
 /**
  * Sample file demonstrating what the CSS may look like
  *
- * - 📦: Is prefixed to host elements and is the replacement for the `:host` selector.
- * - 🏷️: Is prefixed to all other elements.
+ * - 💎: Is prefixed to host elements and is the replacement for the `:host` selector.
+ * - ⭐️: Is prefixed to all other elements.
  */
 
-📦ABC123 {
+💎ABC123 {
 }
 
-🏷️ABC123 {
+⭐️ABC123 {
 }

src/core/object/store.public.ts

@@ -9,7 +9,7 @@ import { snapshotState } from './store';
  * @public
  */
 export function snapshot(elmOrDoc: Element | Document) {
-  const doc = isDocument(elmOrDoc) ? elmOrDoc : getDocument(elmOrDoc);
+  const doc = getDocument(elmOrDoc);
   const data = snapshotState(elmOrDoc);
   const parentJSON = isDocument(elmOrDoc) ? elmOrDoc.body : elmOrDoc;
   const script = doc.createElement('script');

src/core/object/store.ts

@@ -6,7 +6,13 @@ import { getContext } from '../props/props';
 import { getDocument } from '../util/dom';
 import { isDocument, isElement } from '../util/element';
 import { logError, logWarn } from '../util/log';
-import { ELEMENT_ID, ELEMENT_ID_PREFIX, QHostAttr, QObjAttr } from '../util/markers';
+import {
+  ELEMENT_ID,
+  ELEMENT_ID_PREFIX,
+  QContainerAttr,
+  QHostAttr,
+  QObjAttr,
+} from '../util/markers';
 import { qDev } from '../util/qdev';
 import {
   getProxyMap,
@@ -34,7 +40,7 @@ export function resume(elmOrDoc: Element | Document) {
     // logWarn('Skipping hydration because parent element is not q:container');
     return;
   }
-  const doc = isDocument(elmOrDoc) ? elmOrDoc : getDocument(elmOrDoc);
+  const doc = getDocument(elmOrDoc);
   const isDoc = isDocument(elmOrDoc) || elmOrDoc === doc.documentElement;
   const parentJSON = isDoc ? doc.body : parentElm;
   const script = getQwikJSON(parentJSON);
@@ -90,7 +96,7 @@ export function resume(elmOrDoc: Element | Document) {
 }
 
 export function snapshotState(elmOrDoc: Element | Document) {
-  const doc = isDocument(elmOrDoc) ? elmOrDoc : getDocument(elmOrDoc);
+  const doc = getDocument(elmOrDoc);
   const parentElm = isDocument(elmOrDoc) ? elmOrDoc.documentElement : elmOrDoc;
   const proxyMap = getProxyMap(doc);
   const objSet = new Set<any>();
@@ -427,7 +433,7 @@ export function isProxy(obj: any): boolean {
 }
 
 export function isContainer(el: Element) {
-  return el.hasAttribute('q:container');
+  return el.hasAttribute(QContainerAttr);
 }
 
 function hasQObj(el: Element) {

src/core/platform/platform.ts

@@ -1,5 +1,5 @@
+import { getContainer } from '../use/use-core';
 import { getDocument } from '../util/dom';
-import { isDocument } from '../util/element';
 import type { CorePlatform } from './types';
 
 export const createPlatform = (doc: Document): CorePlatform => {
@@ -52,27 +52,10 @@ export const createPlatform = (doc: Document): CorePlatform => {
  * @param url - relative URL
  * @returns fully qualified URL.
  */
-export function toUrl(doc: Document, element: Element | null, url?: string | URL): URL {
-  let _url: string | URL;
-  let _base: string | URL | undefined = undefined;
-
-  if (url === undefined) {
-    //  recursive call
-    if (element) {
-      _url = element.getAttribute('q:base')!;
-      _base = toUrl(
-        doc,
-        element.parentNode && (element.parentNode as HTMLElement).closest('[q\\:base]')
-      );
-    } else {
-      _url = doc.baseURI;
-    }
-  } else if (url) {
-    (_url = url), (_base = toUrl(doc, element!.closest('[q\\:base]')));
-  } else {
-    throw new Error('INTERNAL ERROR');
-  }
-  return new URL(String(_url), _base);
+export function toUrl(doc: Document, element: Element, url: string | URL): URL {
+  const containerEl = getContainer(element);
+  const base = new URL(containerEl?.getAttribute('q:base') ?? doc.baseURI, doc.baseURI);
+  return new URL(url, base);
 }
 
 /**
@@ -85,7 +68,7 @@ export const setPlatform = (doc: Document, plt: CorePlatform) =>
  * @public
  */
 export const getPlatform = (docOrNode: Document | Node) => {
-  const doc = (isDocument(docOrNode) ? docOrNode : getDocument(docOrNode)!) as PlatformDocument;
+  const doc = getDocument(docOrNode) as PlatformDocument;
   return doc[DocumentPlatform] || (doc[DocumentPlatform] = createPlatform(doc));
 };
 

src/core/props/props-on.ts

@@ -25,21 +25,6 @@ export function isOn$Prop(prop: string): boolean {
   return ON$_PROP_REGEX.test(prop);
 }
 
-/**
- * In the case of a component, it is necessary to have `on:q-render` value.
- * However the `component` can run when parent component is rendering only to
- * realize that `on:q-render` already exists. This interface exists to solve that
- * problem.
- *
- * A parent component's `component` returns a `qrlFactory` for `on:q-render`. The
- * `getProps` than looks to see if it already has a resolved value, and if so the
- * `qrlFactory` is ignored, otherwise the `qrlFactory` is used to recover the `QRLInternal`.
- */
-export interface qrlFactory {
-  __brand__: `QRLFactory`;
-  (element: Element): Promise<QRLInternal<any>>;
-}
-
 export function qPropReadQRL(
   ctx: QContext,
   prop: string

src/core/props/props.ts

@@ -4,8 +4,6 @@ import { getProxyMap, readWriteProxy } from '../object/q-object';
 import { resume } from '../object/store';
 import type { RenderContext } from '../render/cursor';
 import { getDocument } from '../util/dom';
-import { isDocument } from '../util/element';
-import { logWarn } from '../util/log';
 import { newQObjectMap, QObjectMap } from './props-obj-map';
 import { qPropWriteQRL, qPropReadQRL } from './props-on';
 import type { QRLInternal } from '../import/qrl-class';
@@ -15,17 +13,11 @@ Error.stackTraceLimit = 9999;
 const Q_IS_RESUMED = '__isResumed__';
 const Q_CTX = '__ctx__';
 
-export function resumeIfNeeded(elm: Element | Document): void {
-  const doc = isDocument(elm) ? elm : getDocument(elm);
-  const root = isDocument(elm) ? elm : elm.closest('[q\\:container]') ?? doc;
-  if (!root) {
-    logWarn('cant find qwik app root');
-    return;
-  }
-  const isHydrated = (root as any)[Q_IS_RESUMED];
+export function resumeIfNeeded(containerEl: Element): void {
+  const isHydrated = (containerEl as any)[Q_IS_RESUMED];
   if (!isHydrated) {
-    (root as any)[Q_IS_RESUMED] = true;
-    resume(root);
+    (containerEl as any)[Q_IS_RESUMED] = true;
+    resume(containerEl);
   }
 }