Skip to content

Commit 58703d6

Browse files
authored
Address context fragments issue (#4717)
* Address context fragments issue * Golf bytesize * Add one more test
1 parent ecf6c40 commit 58703d6

File tree

2 files changed

+174
-3
lines changed

2 files changed

+174
-3
lines changed

src/diff/index.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ export function diff(
258258

259259
let isTopLevelFragment =
260260
tmp != NULL && tmp.type === Fragment && tmp.key == NULL;
261-
let renderResult = isTopLevelFragment ? tmp.props.children : tmp;
261+
let renderResult = tmp;
262262

263263
if (isTopLevelFragment) {
264-
tmp.props.children = NULL;
264+
renderResult = cloneNode(tmp.props.children);
265265
}
266266

267267
oldDom = diffChildren(
@@ -368,6 +368,18 @@ export function commitRoot(commitQueue, root, refQueue) {
368368
});
369369
}
370370

371+
function cloneNode(node) {
372+
if (typeof node !== 'object' || node == NULL) {
373+
return node;
374+
}
375+
376+
if (isArray(node)) {
377+
return node.map(cloneNode);
378+
}
379+
380+
return assign({}, node);
381+
}
382+
371383
/**
372384
* Diff two virtual nodes representing DOM element
373385
* @param {PreactElement} dom The DOM element representing the virtual nodes

test/browser/fragments.test.js

+160-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { setupRerender } from 'preact/test-utils';
2-
import { createElement, render, Component, Fragment } from 'preact';
2+
import {
3+
createElement,
4+
render,
5+
Component,
6+
Fragment,
7+
createContext
8+
} from 'preact';
39
import { setupScratch, teardown } from '../_util/helpers';
410
import { span, div, ul, ol, li, section } from '../_util/dom';
511
import { logCall, clearLog, getLog } from '../_util/logCall';
@@ -3150,4 +3156,157 @@ describe('Fragment', () => {
31503156
expect(scratch.innerHTML).to.equal([div('A'), div(1), div(2)].join(''));
31513157
expectDomLogToBe(['<div>B.remove()', '<div>2A1.appendChild(<div>2)']);
31523158
});
3159+
3160+
it('Should retain content when parent rerenders', () => {
3161+
let toggle;
3162+
const ctx = createContext(null);
3163+
3164+
class Provider extends Component {
3165+
constructor(props) {
3166+
super(props);
3167+
this.state = { value: props.value };
3168+
toggle = () => this.setState({ value: props.value + 1 });
3169+
}
3170+
3171+
render(props) {
3172+
return (
3173+
<ctx.Provider value={this.state.value}>{props.children}</ctx.Provider>
3174+
);
3175+
}
3176+
}
3177+
3178+
class App extends Component {
3179+
render() {
3180+
return (
3181+
<Provider value={1}>
3182+
<Fragment>
3183+
<p>Hello world</p>
3184+
</Fragment>
3185+
</Provider>
3186+
);
3187+
}
3188+
}
3189+
3190+
render(<App />, scratch);
3191+
expect(scratch.innerHTML).to.equal('<p>Hello world</p>');
3192+
3193+
toggle();
3194+
rerender();
3195+
expect(scratch.innerHTML).to.equal('<p>Hello world</p>');
3196+
});
3197+
3198+
it('Should retain content when parent rerenders w/ multiple children', () => {
3199+
let toggle;
3200+
const ctx = createContext(null);
3201+
3202+
class Provider extends Component {
3203+
constructor(props) {
3204+
super(props);
3205+
this.state = { value: props.value };
3206+
toggle = () => this.setState({ value: props.value + 1 });
3207+
}
3208+
3209+
render(props) {
3210+
return (
3211+
<ctx.Provider value={this.state.value}>{props.children}</ctx.Provider>
3212+
);
3213+
}
3214+
}
3215+
3216+
class App extends Component {
3217+
render() {
3218+
return (
3219+
<Provider value={1}>
3220+
<Fragment>
3221+
<p>Hello</p>
3222+
<p>World</p>
3223+
</Fragment>
3224+
</Provider>
3225+
);
3226+
}
3227+
}
3228+
3229+
render(<App />, scratch);
3230+
expect(scratch.innerHTML).to.equal('<p>Hello</p><p>World</p>');
3231+
3232+
toggle();
3233+
rerender();
3234+
expect(scratch.innerHTML).to.equal('<p>Hello</p><p>World</p>');
3235+
});
3236+
3237+
it('Should retain content when parent rerenders w/ text children', () => {
3238+
let toggle;
3239+
const ctx = createContext(null);
3240+
3241+
class Provider extends Component {
3242+
constructor(props) {
3243+
super(props);
3244+
this.state = { value: props.value };
3245+
toggle = () => this.setState({ value: props.value + 1 });
3246+
}
3247+
3248+
render(props) {
3249+
return (
3250+
<ctx.Provider value={this.state.value}>{props.children}</ctx.Provider>
3251+
);
3252+
}
3253+
}
3254+
3255+
class App extends Component {
3256+
render() {
3257+
return (
3258+
<Provider value={1}>
3259+
<Fragment>Hello world</Fragment>
3260+
</Provider>
3261+
);
3262+
}
3263+
}
3264+
3265+
render(<App />, scratch);
3266+
expect(scratch.innerHTML).to.equal('Hello world');
3267+
3268+
toggle();
3269+
rerender();
3270+
expect(scratch.innerHTML).to.equal('Hello world');
3271+
});
3272+
3273+
it('Should retain content when parent rerenders when deeper children are subscribed', () => {
3274+
let toggle;
3275+
const ctx = createContext(null);
3276+
3277+
class Provider extends Component {
3278+
constructor(props) {
3279+
super(props);
3280+
this.state = { value: props.value };
3281+
toggle = () => this.setState({ value: props.value + 1 });
3282+
}
3283+
3284+
render(props) {
3285+
return (
3286+
<ctx.Provider value={this.state.value}>{props.children}</ctx.Provider>
3287+
);
3288+
}
3289+
}
3290+
3291+
class App extends Component {
3292+
render() {
3293+
return (
3294+
<Provider value={1}>
3295+
<Fragment>
3296+
<ctx.Consumer>
3297+
{value => <p>I can count to {value}</p>}
3298+
</ctx.Consumer>
3299+
</Fragment>
3300+
</Provider>
3301+
);
3302+
}
3303+
}
3304+
3305+
render(<App />, scratch);
3306+
expect(scratch.innerHTML).to.equal('<p>I can count to 1</p>');
3307+
3308+
toggle();
3309+
rerender();
3310+
expect(scratch.innerHTML).to.equal('<p>I can count to 2</p>');
3311+
});
31533312
});

0 commit comments

Comments
 (0)