Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for rendering comment nodes #3957

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const COMMENT_TYPE = 8;
export const EMPTY_OBJ = {};
export const EMPTY_ARR = [];
export const IS_NON_DIMENSIONAL =
Expand Down
5 changes: 5 additions & 0 deletions src/create-element.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { slice } from './util';
import options from './options';
import { COMMENT_TYPE } from './constants';

let vnodeId = 0;

Expand All @@ -12,6 +13,10 @@ let vnodeId = 0;
* @returns {import('./internal').VNode}
*/
export function createElement(type, props, children) {
if (type === '!--') {
return createVNode(COMMENT_TYPE, children, null, null);
}

let normalizedProps = {},
key,
ref,
Expand Down
14 changes: 10 additions & 4 deletions src/diff/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EMPTY_OBJ } from '../constants';
import { COMMENT_TYPE, EMPTY_OBJ } from '../constants';
import { Component, getDomSibling } from '../component';
import { Fragment } from '../create-element';
import { diffChildren } from './children';
Expand Down Expand Up @@ -355,8 +355,11 @@ function diffElementNodes(
// excessDomChildren so it isn't later removed in diffChildren
if (
child &&
'setAttribute' in child === !!nodeType &&
(nodeType ? child.localName === nodeType : child.nodeType === 3)
(nodeType === COMMENT_TYPE
? child.nodeType === COMMENT_TYPE
: nodeType
? child.localName === nodeType
: child.nodeType === 3)
) {
dom = child;
excessDomChildren[i] = null;
Expand All @@ -369,6 +372,9 @@ function diffElementNodes(
if (nodeType === null) {
// @ts-ignore createTextNode returns Text, we expect PreactElement
return document.createTextNode(newProps);
} else if (nodeType === COMMENT_TYPE) {
// @ts-ignore createComment returns Comment, we expect PreactElement
return document.createComment(newProps);
}

if (isSvg) {
Expand All @@ -391,7 +397,7 @@ function diffElementNodes(
isHydrating = false;
}

if (nodeType === null) {
if (nodeType === null || nodeType === COMMENT_TYPE) {
// During hydration, we still have to split merged text from SSR'd HTML.
if (oldProps !== newProps && (!isHydrating || dom.data !== newProps)) {
dom.data = newProps;
Expand Down
6 changes: 6 additions & 0 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ export abstract class Component<P, S> {
// Preact createElement
// -----------------------------------

export function createElement(
type: '!--',
props: unknown,
children: string
): VNode<any>;
export function createElement(
type: 'input',
props:
Expand Down Expand Up @@ -229,6 +234,7 @@ export namespace createElement {
export import JSX = JSXInternal;
}

export function h(type: '!--', props: unknown, children: string): VNode<any>;
export function h(
type: 'input',
props:
Expand Down
4 changes: 3 additions & 1 deletion src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,11 @@ type RefObject<T> = { current: T | null };
type RefCallback<T> = { (instance: T | null): void; current: undefined };
type Ref<T> = RefObject<T> | RefCallback<T>;

type COMMENT_TYPE = 8;

export interface VNode<P = {}> extends preact.VNode<P> {
// Redefine type here using our internal ComponentType type
type: string | ComponentType<P>;
type: string | ComponentType<P> | COMMENT_TYPE;
props: P & { children: ComponentChildren };
ref?: Ref<any> | null;
_children: Array<VNode<any>> | null;
Expand Down
143 changes: 143 additions & 0 deletions test/browser/comments.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { h, createElement, render, hydrate, Fragment } from 'preact';
import { setupScratch, teardown } from '../_util/helpers';

/** @jsx createElement */
/** @jsxFrag Fragment */

const COMMENT = '!--';

describe('keys', () => {
/** @type {HTMLDivElement} */
let scratch;

beforeEach(() => {
scratch = setupScratch();
});

afterEach(() => {
teardown(scratch);
});

it('should not render comments', () => {
render(h(COMMENT, null, 'test'), scratch);
expect(scratch.innerHTML).to.equal('<!--test-->');
});

it('should render comments in elements', () => {
render(<div>{h(COMMENT, null, 'test')}</div>, scratch);
expect(scratch.innerHTML).to.equal('<div><!--test--></div>');
});

it('should render Components that return comments', () => {
function App() {
return h(COMMENT, null, 'test');
}
render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<!--test-->');
});

it('should render Fragments that wrap comments', () => {
function App() {
return <Fragment>{h(COMMENT, null, 'test')}</Fragment>;
}
render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<!--test-->');
});

it('should render components that use comments to delimit start and end of a component', () => {
function App() {
return (
<div>
{h(COMMENT, null, 'start')}
<div>test</div>
{h(COMMENT, null, 'end')}
</div>
);
}
render(<App />, scratch);
expect(scratch.innerHTML).to.equal(
'<div><!--start--><div>test</div><!--end--></div>'
);
});

it('should render components that use comments to delimit start and end of a component with a Fragment', () => {
function App() {
return (
<Fragment>
{h(COMMENT, null, 'start')}
<div>test</div>
{h(COMMENT, null, 'end')}
</Fragment>
);
}
render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<!--start--><div>test</div><!--end-->');
});

it('should move comments to the correct location when moving a component', () => {
function Child() {
return (
<>
{h(COMMENT, null, 'start')}
<div>test</div>
{h(COMMENT, null, 'end')}
</>
);
}

/** @type {(props: { move?: boolean }) => any} */
function App({ move = false }) {
if (move) {
return [
<div key="a">a</div>,
<Child key="child" />,
<div key="b">b</div>
];
}

return [
<Child key="child" />,
<div key="a">a</div>,
<div key="b">b</div>
];
}

const childHTML = '<!--start--><div>test</div><!--end-->';

render(<App />, scratch);
expect(scratch.innerHTML).to.equal(`${childHTML}<div>a</div><div>b</div>`);

render(<App move />, scratch);
expect(scratch.innerHTML).to.equal(`<div>a</div>${childHTML}<div>b</div>`);

render(<App />, scratch);
expect(scratch.innerHTML).to.equal(`${childHTML}<div>a</div><div>b</div>`);
});

it('should correctly show hide DOM around comments', () => {
function App({ show = false }) {
return (
<>
{h(COMMENT, null, 'start')}
{show && <div>test</div>}
{h(COMMENT, null, 'end')}
</>
);
}

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<!--start--><!--end-->');

render(<App show />, scratch);
expect(scratch.innerHTML).to.equal('<!--start--><div>test</div><!--end-->');

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<!--start--><!--end-->');
});

it('should hydrate comments VNodes', () => {
scratch.innerHTML = '<div><!--test--></div>';
hydrate(<div>{h(COMMENT, null, 'test')}</div>, scratch);
expect(scratch.innerHTML).to.equal('<div><!--test--></div>');
});
});