Skip to content

Commit

Permalink
Fix options hook calling order (#238)
Browse files Browse the repository at this point in the history 
* Fix options hook calling order (diff -> children -> diffed), add comments

* Create honest-kangaroos-mix.md

* update options hooks tests to reflect correct calling order
  • Loading branch information
@developit
developit authored Aug 19, 2022
1 parent fa53a96 commit 7cdf4d6
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 79 deletions.
5 changes: 5 additions & 0 deletions .changeset/honest-kangaroos-mix.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"preact-render-to-string": patch
---

Fix the order of invocation for the "before diff" (`__b`) and "diffed" [options hooks](https://preactjs.com/guide/v10/options/).
89 changes: 26 additions & 63 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,17 @@ const assign = Object.assign;

/** The default export is an alias of `render()`. */
function _renderToString(vnode, context, isSvgMode, selectValue) {
// Ignore non-rendered VNodes/values
if (vnode == null || vnode === true || vnode === false || vnode === '') {
return '';
}

// #text nodes
// Text VNodes: escape as HTML
if (typeof vnode !== 'object') {
return encodeEntities(vnode);
}

// Recurse into children / Arrays
if (isArray(vnode)) {
let rendered = '';
for (let i = 0; i < vnode.length; i++) {
Expand All @@ -200,13 +202,15 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
return rendered;
}

let nodeName = vnode.type,
if (options[DIFF]) options[DIFF](vnode);

let type = vnode.type,
props = vnode.props;
const isComponent = typeof nodeName === 'function';

// components
// Invoke rendering on Components
const isComponent = typeof type === 'function';
if (isComponent) {
if (nodeName === Fragment) {
if (type === Fragment) {
return _renderToString(
vnode.props.children,
context,
Expand All @@ -215,10 +219,8 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
);
}

if (options[DIFF]) options[DIFF](vnode);

let rendered;
if (nodeName.prototype && typeof nodeName.prototype.render === 'function') {
if (type.prototype && typeof type.prototype.render === 'function') {
rendered = renderClassComponent(vnode, context);
} else {
rendered = renderFunctionComponent(vnode, context);
Expand All @@ -229,53 +231,24 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
context = assign({}, context, component.getChildContext());
}

// Recurse into children before invoking the after-diff hook
const str = _renderToString(rendered, context, isSvgMode, selectValue);
if (options[DIFFED]) options[DIFFED](vnode);

return _renderToString(rendered, context, isSvgMode, selectValue);
return str;
}

// render JSX to HTML
// Serialize Element VNodes to HTML
let s = '<',
children,
html;

s = s + nodeName;
s = s + type;

if (props) {
children = props.children;
for (let name in props) {
let v = props[name];

// switch (name) {
// case 'className':
// if ('class' in props) continue;
// name = 'class';
// break;
// case 'htmlFor':
// if ('for' in props) continue;
// name = 'for';
// break;
// case 'defaultValue':
// name = 'value';
// break;
// case 'defaultChecked':
// name = 'checked';
// break;
// case 'defaultSelected':
// name = 'selected';
// break;
// case 'key':
// case 'ref':
// case '__self':
// case '__source':
// case 'children':
// continue;
// default:
// if (isSvgMode && XLINK.test(name)) {
// name = name.toLowerCase().replace(/^xlink:?/, 'xlink:');
// }
// }

if (
name === 'key' ||
name === 'ref' ||
Expand All @@ -295,10 +268,9 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {

if (name === 'dangerouslySetInnerHTML') {
html = v && v.__html;
} else if (nodeName === 'textarea' && name === 'value') {
} else if (type === 'textarea' && name === 'value') {
// <textarea value="a&b"> --> <textarea>a&amp;b</textarea>
children = v;
// html = encodeEntities(v);
} else if ((v || v === 0 || v === '') && typeof v !== 'function') {
if (v === true || v === '') {
v = name;
Expand All @@ -307,12 +279,12 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
}

if (name === 'value') {
if (nodeName === 'select') {
if (type === 'select') {
selectValue = v;
continue;
} else if (
// If we're looking at an <option> and it's the currently selected one
nodeName === 'option' &&
type === 'option' &&
selectValue == v &&
// and the <option> doesn't already have a selected attribute on it
!('selected' in props)
Expand All @@ -328,25 +300,17 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
let startElement = s;
s = s + '>';

if (UNSAFE_NAME.test(nodeName)) {
throw new Error(`${nodeName} is not a valid HTML tag name in ${s}`);
if (UNSAFE_NAME.test(type)) {
throw new Error(`${type} is not a valid HTML tag name in ${s}`);
}

let pieces = '';
let hasChildren = false;

// let children = isArray(propChildren)
// ? propChildren
// : propChildren != null
// ? [propChildren]
// : undefined;
if (html) {
// return s + html + '</' + nodeName + '>';
// s = s + html;
pieces = pieces + html;
hasChildren = true;
} else if (typeof children === 'string') {
// s = s + encodeEntities(children);
pieces = pieces + encodeEntities(children);
hasChildren = true;
} else if (isArray(children)) {
Expand All @@ -355,38 +319,37 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {

if (child != null && child !== false) {
let childSvgMode =
nodeName === 'svg' || (nodeName !== 'foreignObject' && isSvgMode);
type === 'svg' || (type !== 'foreignObject' && isSvgMode);
let ret = _renderToString(child, context, childSvgMode, selectValue);

// Skip if we received an empty string
if (ret) {
// s = s + ret;
pieces = pieces + ret;
hasChildren = true;
}
}
}
} else if (children != null && children !== false && children !== true) {
let childSvgMode =
nodeName === 'svg' || (nodeName !== 'foreignObject' && isSvgMode);
type === 'svg' || (type !== 'foreignObject' && isSvgMode);
let ret = _renderToString(children, context, childSvgMode, selectValue);

// Skip if we received an empty string
if (ret) {
// s = s + ret;
pieces = pieces + ret;
hasChildren = true;
}
}

if (options[DIFFED]) options[DIFFED](vnode);

if (hasChildren) {
s = s + pieces;
// return s + pieces + '</' + nodeName + '>';
} else if (VOID_ELEMENTS.test(nodeName)) {
} else if (VOID_ELEMENTS.test(type)) {
return startElement + ' />';
}

return s + '</' + nodeName + '>';
return s + '</' + type + '>';
}

/** The default export is an alias of `render()`. */
Expand Down
40 changes: 24 additions & 16 deletions test/render.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1154,30 +1154,38 @@ describe('render', () => {
if (oldCommit) oldCommit(...args);
};

function Component1({ children }) {
return children;
}
const outer = <Outer />;
const inner = <Inner />;
const div = <div />;

function Component2() {
return <div />;
function Outer() {
return inner;
}

const vnode2 = <Component2>1</Component2>;
const vnode1 = <Component1>{vnode2}</Component1>;
function Inner() {
return div;
}

render(vnode1);
render(outer);

expect(calls).to.deep.equal([
['_diff', [vnode1]],
['_render', [vnode1]],
['diffed', [vnode1]],
['_diff', [vnode2]],
['_render', [vnode2]],
['diffed', [vnode2]],
['_commit', [vnode1, []]]
['_diff', [outer]], // before diff
['_render', [outer]], // render attempt

['_diff', [inner]], // before diff
['_render', [inner]], // render attempt

// innermost <div>
['_diff', [div]], // before diff
['diffed', [div]], // after diff

['diffed', [inner]], // after diff
['diffed', [outer]], // after diff

['_commit', [outer, []]] // commit root
]);

expect(calls.length).to.equal(7);
expect(calls.length).to.equal(9);

options.__b = oldDiff;
options.__r = oldRender;
Expand Down

0 comments on commit 7cdf4d6

Please sign in to comment.