Skip to content

Commit 7cdf4d6

Browse files
authored
Fix options hook calling order (#238)
* Fix options hook calling order (diff -> children -> diffed), add comments * Create honest-kangaroos-mix.md * update options hooks tests to reflect correct calling order
1 parent fa53a96 commit 7cdf4d6

File tree

3 files changed

+55
-79
lines changed

3 files changed

+55
-79
lines changed

.changeset/honest-kangaroos-mix.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"preact-render-to-string": patch
3+
---
4+
5+
Fix the order of invocation for the "before diff" (`__b`) and "diffed" [options hooks](https://preactjs.com/guide/v10/options/).

src/index.js

Lines changed: 26 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -182,15 +182,17 @@ const assign = Object.assign;
182182

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

189-
// #text nodes
190+
// Text VNodes: escape as HTML
190191
if (typeof vnode !== 'object') {
191192
return encodeEntities(vnode);
192193
}
193194

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

203-
let nodeName = vnode.type,
205+
if (options[DIFF]) options[DIFF](vnode);
206+
207+
let type = vnode.type,
204208
props = vnode.props;
205-
const isComponent = typeof nodeName === 'function';
206209

207-
// components
210+
// Invoke rendering on Components
211+
const isComponent = typeof type === 'function';
208212
if (isComponent) {
209-
if (nodeName === Fragment) {
213+
if (type === Fragment) {
210214
return _renderToString(
211215
vnode.props.children,
212216
context,
@@ -215,10 +219,8 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
215219
);
216220
}
217221

218-
if (options[DIFF]) options[DIFF](vnode);
219-
220222
let rendered;
221-
if (nodeName.prototype && typeof nodeName.prototype.render === 'function') {
223+
if (type.prototype && typeof type.prototype.render === 'function') {
222224
rendered = renderClassComponent(vnode, context);
223225
} else {
224226
rendered = renderFunctionComponent(vnode, context);
@@ -229,53 +231,24 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
229231
context = assign({}, context, component.getChildContext());
230232
}
231233

234+
// Recurse into children before invoking the after-diff hook
235+
const str = _renderToString(rendered, context, isSvgMode, selectValue);
232236
if (options[DIFFED]) options[DIFFED](vnode);
233-
234-
return _renderToString(rendered, context, isSvgMode, selectValue);
237+
return str;
235238
}
236239

237-
// render JSX to HTML
240+
// Serialize Element VNodes to HTML
238241
let s = '<',
239242
children,
240243
html;
241244

242-
s = s + nodeName;
245+
s = s + type;
243246

244247
if (props) {
245248
children = props.children;
246249
for (let name in props) {
247250
let v = props[name];
248251

249-
// switch (name) {
250-
// case 'className':
251-
// if ('class' in props) continue;
252-
// name = 'class';
253-
// break;
254-
// case 'htmlFor':
255-
// if ('for' in props) continue;
256-
// name = 'for';
257-
// break;
258-
// case 'defaultValue':
259-
// name = 'value';
260-
// break;
261-
// case 'defaultChecked':
262-
// name = 'checked';
263-
// break;
264-
// case 'defaultSelected':
265-
// name = 'selected';
266-
// break;
267-
// case 'key':
268-
// case 'ref':
269-
// case '__self':
270-
// case '__source':
271-
// case 'children':
272-
// continue;
273-
// default:
274-
// if (isSvgMode && XLINK.test(name)) {
275-
// name = name.toLowerCase().replace(/^xlink:?/, 'xlink:');
276-
// }
277-
// }
278-
279252
if (
280253
name === 'key' ||
281254
name === 'ref' ||
@@ -295,10 +268,9 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
295268

296269
if (name === 'dangerouslySetInnerHTML') {
297270
html = v && v.__html;
298-
} else if (nodeName === 'textarea' && name === 'value') {
271+
} else if (type === 'textarea' && name === 'value') {
299272
// <textarea value="a&b"> --> <textarea>a&amp;b</textarea>
300273
children = v;
301-
// html = encodeEntities(v);
302274
} else if ((v || v === 0 || v === '') && typeof v !== 'function') {
303275
if (v === true || v === '') {
304276
v = name;
@@ -307,12 +279,12 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
307279
}
308280

309281
if (name === 'value') {
310-
if (nodeName === 'select') {
282+
if (type === 'select') {
311283
selectValue = v;
312284
continue;
313285
} else if (
314286
// If we're looking at an <option> and it's the currently selected one
315-
nodeName === 'option' &&
287+
type === 'option' &&
316288
selectValue == v &&
317289
// and the <option> doesn't already have a selected attribute on it
318290
!('selected' in props)
@@ -328,25 +300,17 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
328300
let startElement = s;
329301
s = s + '>';
330302

331-
if (UNSAFE_NAME.test(nodeName)) {
332-
throw new Error(`${nodeName} is not a valid HTML tag name in ${s}`);
303+
if (UNSAFE_NAME.test(type)) {
304+
throw new Error(`${type} is not a valid HTML tag name in ${s}`);
333305
}
334306

335307
let pieces = '';
336308
let hasChildren = false;
337309

338-
// let children = isArray(propChildren)
339-
// ? propChildren
340-
// : propChildren != null
341-
// ? [propChildren]
342-
// : undefined;
343310
if (html) {
344-
// return s + html + '</' + nodeName + '>';
345-
// s = s + html;
346311
pieces = pieces + html;
347312
hasChildren = true;
348313
} else if (typeof children === 'string') {
349-
// s = s + encodeEntities(children);
350314
pieces = pieces + encodeEntities(children);
351315
hasChildren = true;
352316
} else if (isArray(children)) {
@@ -355,38 +319,37 @@ function _renderToString(vnode, context, isSvgMode, selectValue) {
355319

356320
if (child != null && child !== false) {
357321
let childSvgMode =
358-
nodeName === 'svg' || (nodeName !== 'foreignObject' && isSvgMode);
322+
type === 'svg' || (type !== 'foreignObject' && isSvgMode);
359323
let ret = _renderToString(child, context, childSvgMode, selectValue);
360324

361325
// Skip if we received an empty string
362326
if (ret) {
363-
// s = s + ret;
364327
pieces = pieces + ret;
365328
hasChildren = true;
366329
}
367330
}
368331
}
369332
} else if (children != null && children !== false && children !== true) {
370333
let childSvgMode =
371-
nodeName === 'svg' || (nodeName !== 'foreignObject' && isSvgMode);
334+
type === 'svg' || (type !== 'foreignObject' && isSvgMode);
372335
let ret = _renderToString(children, context, childSvgMode, selectValue);
373336

374337
// Skip if we received an empty string
375338
if (ret) {
376-
// s = s + ret;
377339
pieces = pieces + ret;
378340
hasChildren = true;
379341
}
380342
}
381343

344+
if (options[DIFFED]) options[DIFFED](vnode);
345+
382346
if (hasChildren) {
383347
s = s + pieces;
384-
// return s + pieces + '</' + nodeName + '>';
385-
} else if (VOID_ELEMENTS.test(nodeName)) {
348+
} else if (VOID_ELEMENTS.test(type)) {
386349
return startElement + ' />';
387350
}
388351

389-
return s + '</' + nodeName + '>';
352+
return s + '</' + type + '>';
390353
}
391354

392355
/** The default export is an alias of `render()`. */

test/render.test.js

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,30 +1154,38 @@ describe('render', () => {
11541154
if (oldCommit) oldCommit(...args);
11551155
};
11561156

1157-
function Component1({ children }) {
1158-
return children;
1159-
}
1157+
const outer = <Outer />;
1158+
const inner = <Inner />;
1159+
const div = <div />;
11601160

1161-
function Component2() {
1162-
return <div />;
1161+
function Outer() {
1162+
return inner;
11631163
}
11641164

1165-
const vnode2 = <Component2>1</Component2>;
1166-
const vnode1 = <Component1>{vnode2}</Component1>;
1165+
function Inner() {
1166+
return div;
1167+
}
11671168

1168-
render(vnode1);
1169+
render(outer);
11691170

11701171
expect(calls).to.deep.equal([
1171-
['_diff', [vnode1]],
1172-
['_render', [vnode1]],
1173-
['diffed', [vnode1]],
1174-
['_diff', [vnode2]],
1175-
['_render', [vnode2]],
1176-
['diffed', [vnode2]],
1177-
['_commit', [vnode1, []]]
1172+
['_diff', [outer]], // before diff
1173+
['_render', [outer]], // render attempt
1174+
1175+
['_diff', [inner]], // before diff
1176+
['_render', [inner]], // render attempt
1177+
1178+
// innermost <div>
1179+
['_diff', [div]], // before diff
1180+
['diffed', [div]], // after diff
1181+
1182+
['diffed', [inner]], // after diff
1183+
['diffed', [outer]], // after diff
1184+
1185+
['_commit', [outer, []]] // commit root
11781186
]);
11791187

1180-
expect(calls.length).to.equal(7);
1188+
expect(calls.length).to.equal(9);
11811189

11821190
options.__b = oldDiff;
11831191
options.__r = oldRender;

0 commit comments

Comments
 (0)