Skip to content

Commit

Permalink
fix: unprojected content does not lose state (#576)
Browse files Browse the repository at this point in the history
  • Loading branch information
manucorporat authored Jun 8, 2022
1 parent 255d044 commit a4a6b98
Show file tree
Hide file tree
Showing 18 changed files with 121 additions and 128 deletions.
1 change: 0 additions & 1 deletion packages/docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"@builder.io/qwik": "0.0.22",
"@builder.io/qwik-city": "0.0.12",
"@cloudflare/kv-asset-handler": "0.2.0",
"@cloudflare/workers-types": "^3.11.0",
"autoprefixer": "10.4.7",
"fflate": "^0.7.3",
"node-fetch": "2.6.7",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ReplEventMessage } from '../types';
import { QWIK_REPL_RESULT_CACHE } from './repl-constants';
import { sendMessageToReplServer } from './repl-messenger';

export const requestHandler = async (ev: FetchEvent) => {
export const requestHandler = async (ev: any) => {
const reqUrl = new URL(ev.request.url);
const pathname = reqUrl.pathname;
const parts = pathname.split('/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface ReplGlobalApi {

export interface QwikWorkerGlobal extends ReplGlobalApi {
onmessage: (ev: MessageEvent) => void;
onfetch: (ev: FetchEvent) => void;
onfetch: (ev: Event) => void;
oninstall: (ev: any) => void;
onactivate: () => void;
skipWaiting: () => void;
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/src/entry.cloudflare.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render } from './entry.ssr';
import replServerHtml from '@repl-server-html';

export const onRequestGet: PagesFunction = async ({ request, next, waitUntil }) => {
export const onRequestGet = async ({ request, next, waitUntil }: any) => {
try {
const url = new URL(request.url);

Expand Down
4 changes: 2 additions & 2 deletions packages/docs/src/pages/docs/components/projection.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ Results in:
```html
<my-app>
<project>
<template q:slot="">unwrapped text</template>
<q:template q:slot="">unwrapped text</template>
<div></div>
</project>
</my-app>
```

Notice that the un-projected content is moved into inert `<template>`. This is done just in case the `Project` component rerenders and inserts a`<Slot>`. In that case, we don't want to rerender the parent component. The rendering of the two components needs to stay independent.
Notice that the un-projected content is moved into inert `<q:template>`. This is done just in case the `Project` component rerenders and inserts a`<Slot>`. In that case, we don't want to rerender the parent component. The rendering of the two components needs to stay independent.

### Default slot content

Expand Down
54 changes: 33 additions & 21 deletions packages/qwik/src/core/render/cursor.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { OnRenderProp, QHostSelector, QSlotAttr } from '../util/markers';
import { OnRenderProp, QHostAttr, QSlotAttr } from '../util/markers';
import {
cleanupContext,
ComponentCtx,
Expand Down Expand Up @@ -206,20 +206,20 @@ function isComponentNode(node: JSXNode) {
return node.props && OnRenderProp in node.props;
}

function getCh(elm: Node) {
return Array.from(elm.childNodes).filter(isNode);
function getCh(elm: Node, filter: (el: Node) => boolean) {
return Array.from(elm.childNodes).filter(filter);
}

export function getChildren(elm: Node, mode: ChildrenMode): Node[] {
switch (mode) {
case 'default':
return getCh(elm);
return getCh(elm, isNode);
case 'slot':
return getCh(elm).filter(isChildSlot);
return getCh(elm, isChildSlot);
case 'root':
return getCh(elm).filter(isChildComponent);
return getCh(elm, isChildComponent);
case 'fallback':
return getCh(elm).filter(isFallback);
return getCh(elm, isFallback);
}
}
export function isNode(elm: Node): boolean {
Expand All @@ -232,15 +232,15 @@ function isFallback(node: Node): boolean {
}

function isChildSlot(node: Node) {
return node.nodeName !== 'Q:FALLBACK' && isChildComponent(node);
return isNode(node) && node.nodeName !== 'Q:FALLBACK' && node.nodeName !== 'Q:TEMPLATE';
}

function isSlotTemplate(node: Node): node is HTMLTemplateElement {
return node.nodeName === 'TEMPLATE' && (node as Element).hasAttribute(QSlotAttr);
return node.nodeName === 'Q:TEMPLATE';
}

function isChildComponent(node: Node): boolean {
return node.nodeName !== 'TEMPLATE' || !(node as Element).hasAttribute(QSlotAttr);
return isNode(node) && node.nodeName !== 'Q:TEMPLATE';
}

function splitBy<T>(input: T[], condition: (item: T) => string): Record<string, T[]> {
Expand Down Expand Up @@ -405,17 +405,19 @@ function getSlotElement(
}
const templateEl = slotMaps.templates[slotName];
if (templateEl) {
return templateEl.content as any;
return templateEl;
}
const template = createTemplate(ctx, slotName);
prepend(ctx, parentEl, template);
slotMaps.templates[slotName] = template;
return template.content as any;
return template;
}

function createTemplate(ctx: RenderContext, slotName: string) {
const template = createElement(ctx, 'template', false) as HTMLTemplateElement;
const template = createElement(ctx, 'q:template', false);
template.setAttribute(QSlotAttr, slotName);
template.setAttribute('hidden', '');
template.setAttribute('aria-hidden', 'true');
return template;
}

Expand All @@ -441,7 +443,7 @@ export function resolveSlotProjection(
// Move slot to template
const template = createTemplate(ctx, key);
const slotChildren = getChildren(slotEl, 'slot');
template.content.append(...slotChildren);
template.append(...slotChildren);
hostElm.insertBefore(template, hostElm.firstChild);

ctx.operations.push({
Expand All @@ -459,7 +461,7 @@ export function resolveSlotProjection(
// Move template to slot
const template = before.templates[key];
if (template) {
slotEl.appendChild(template.content);
slotEl.append(...getChildren(template, 'default'));
template.remove();
ctx.operations.push({
el: slotEl,
Expand Down Expand Up @@ -549,7 +551,7 @@ function createElm(rctx: RenderContext, vnode: JSXNode, isSvg: boolean): ValueOr
}
interface SlotMaps {
slots: Record<string, Element | undefined>;
templates: Record<string, HTMLTemplateElement | undefined>;
templates: Record<string, Element | undefined>;
}

const getSlots = (componentCtx: ComponentCtx | undefined, hostElm: Element): SlotMaps => {
Expand Down Expand Up @@ -825,11 +827,7 @@ function removeNode(ctx: RenderContext, el: Node) {
const parent = el.parentNode;
if (parent) {
if (el.nodeType === 1) {
const subsManager = ctx.containerState.subsManager;
cleanupElement(el as Element, subsManager);
(el as Element)
.querySelectorAll(QHostSelector)
.forEach((el) => cleanupElement(el, subsManager));
cleanupTree(el as Element, ctx.containerState.subsManager);
}
parent.removeChild(el);
} else if (qDev) {
Expand All @@ -844,6 +842,20 @@ function removeNode(ctx: RenderContext, el: Node) {
});
}

export function cleanupTree(parent: Element, subsManager: SubscriptionManager) {
if (parent.nodeName === 'Q:SLOT') {
return;
}
if (parent.hasAttribute(QHostAttr)) {
cleanupElement(parent, subsManager);
}
let child = parent.firstElementChild;
while (child) {
cleanupTree(child, subsManager);
child = child.nextElementSibling;
}
}

function cleanupElement(el: Element, subsManager: SubscriptionManager) {
const ctx = tryGetContext(el);
if (ctx) {
Expand Down
2 changes: 1 addition & 1 deletion packages/qwik/src/core/render/render.public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function injectQwikSlotCSS(docOrElm: Document | Element) {
const element = isDocument(docOrElm) ? docOrElm.head : docOrElm;
const style = doc.createElement('style');
style.setAttribute('id', 'qwik/base-styles');
style.textContent = `q\\:slot{display:contents}q\\:fallback{display:none}q\\:fallback:last-child{display:contents}`;
style.textContent = `q\\:slot{display:contents}q\\:fallback,q\\:template{display:none}q\\:fallback:last-child{display:contents}`;
element.insertBefore(style, element.firstChild);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/qwik/src/core/render/render.unit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,9 @@ describe('render', () => {
);
expectRendered(
<project>
<template q:slot="ignore">
<q:template q:slot="ignore" aria-hidden="true" hidden>
<span q:slot="ignore">IGNORE</span>
</template>
</q:template>
<section>
<q:slot>
<q:fallback>..default..</q:fallback>
Expand Down
22 changes: 0 additions & 22 deletions packages/qwik/src/core/util/dom.unit.ts

This file was deleted.

36 changes: 0 additions & 36 deletions packages/qwik/src/core/util/types.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,3 @@
import { QHostAttr, QSlotAttr } from './markers';

/**
* Returns true if the `node` is `Element` and of the right `tagName`.
*
* @param node
* @private
*/
export function isDomElementWithTagName(
node: Node | null | undefined,
tagName: string
): node is Element {
return isHtmlElement(node) && node.tagName.toUpperCase() == tagName.toUpperCase();
}

/**
* @private
*/
export function isTemplateElement(node: Node | null | undefined): node is HTMLTemplateElement {
return isDomElementWithTagName(node, 'template');
}

/**
* @private
*/
export function isQSLotTemplateElement(node: Node | null | undefined): node is HTMLTemplateElement {
return isTemplateElement(node) && node.hasAttribute(QSlotAttr);
}

/**
* @private
*/
export function isComponentElement(node: Node | null | undefined): node is HTMLElement {
return isHtmlElement(node) && node.hasAttribute(QHostAttr);
}

/**
* @private
*/
Expand Down
37 changes: 36 additions & 1 deletion packages/qwik/src/testing/expect-dom.unit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,42 @@ import type { QwikJSX } from '@builder.io/qwik';
import qwikDom from '@builder.io/qwik-dom';
import { isJSXNode } from '../core/render/jsx/jsx-runtime';
import { isComment, isElement, isText } from '../core/util/element';
import { isTemplateElement } from '../core/util/types';
import { QHostAttr, QSlotAttr } from '../core/util/markers';
import { isHtmlElement } from '../core/util/types';

/**
* Returns true if the `node` is `Element` and of the right `tagName`.
*
* @param node
* @private
*/
export function isDomElementWithTagName(
node: Node | null | undefined,
tagName: string
): node is Element {
return isHtmlElement(node) && node.tagName.toUpperCase() == tagName.toUpperCase();
}

/**
* @private
*/
export function isTemplateElement(node: Node | null | undefined): node is HTMLTemplateElement {
return isDomElementWithTagName(node, 'template');
}

/**
* @private
*/
export function isQSLotTemplateElement(node: Node | null | undefined): node is HTMLTemplateElement {
return isTemplateElement(node) && node.hasAttribute(QSlotAttr);
}

/**
* @private
*/
export function isComponentElement(node: Node | null | undefined): node is HTMLElement {
return isHtmlElement(node) && node.hasAttribute(QHostAttr);
}

export function expectDOM(
actual: Element,
Expand Down
30 changes: 19 additions & 11 deletions starters/apps/e2e/src/components/slot/slot.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { component$, useStore, Slot } from '@builder.io/qwik';
import { component$, useStore, Slot, Host } from '@builder.io/qwik';

export const SlotParent = component$(() => {
const state = useStore({
Expand All @@ -19,7 +19,9 @@ export const SlotParent = component$(() => {
</Button>

<Thing state={state} id="btn3">
<Button state={state}>{!state.removeContent && <>INSIDE THING {state.count}</>}</Button>
<Button host:id="projected" state={state}>
{!state.removeContent && <>INSIDE THING {state.count}</>}
</Button>
</Thing>

<div>
Expand Down Expand Up @@ -60,16 +62,22 @@ export const SlotParent = component$(() => {

export const Button = component$((props: { state: any }) => {
return (
<button class="todoapp">
<Slot name="start">Placeholder Start</Slot>
<Host
onClick$={() => {
props.state.count--;
}}
>
<button class="todoapp">
<Slot name="start">Placeholder Start</Slot>

{!props.state.disableButtons && (
<div>
<Slot />
</div>
)}
<Slot name="end" />
</button>
{!props.state.disableButtons && (
<div>
<Slot />
</div>
)}
<Slot name="end" />
</button>
</Host>
);
});

Expand Down
23 changes: 23 additions & 0 deletions starters/e2e/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,29 @@ test.describe('e2e', () => {
expect((await content2.innerText()).trim()).toEqual('START 1');
expect((await content3.innerText()).trim()).toEqual('Placeholder Start\nINSIDE THING 1');
});

test('should not lose q context', async ({ page }) => {
const content3 = await page.locator('#btn3');
const projected = await page.locator('#projected');
const btnToggleThing = await page.locator('#btn-toggle-thing');
const btnCount = await page.locator('#btn-count');

await btnCount.click();
await page.waitForTimeout(100);
expect((await content3.innerText()).trim()).toEqual('Placeholder Start\nINSIDE THING 1');

// btnToggleButtons
await btnToggleThing.click();
await page.waitForTimeout(100);
await btnToggleThing.click();
await page.waitForTimeout(100);

// Click projected
await projected.click();
await page.waitForTimeout(100);

expect((await content3.innerText()).trim()).toEqual('Placeholder Start\nINSIDE THING 0');
});
});

test.describe('factory', () => {
Expand Down
Loading

0 comments on commit a4a6b98

Please sign in to comment.