preactjs / preact-custom-element

Wrap your component up as a custom element
MIT License
363 stars 52 forks source link

Issues re-rendering CE root with preact #6

Closed majo44 closed 4 years ago

majo44 commented 7 years ago

I do not using shadowDOM

class Clock extends Component<{},{time: number}> {
    private timer: any;
    constructor() {
        super();
        this.state.time = Date.now();
    }

    componentDidMount() {
        this.timer = setInterval(() => this.setState({ time: Date.now() }), 1000);
    }
    componentWillUnmount() {
        clearInterval(this.timer);
    }

    render(props: any, state: {time: number}) {
        let time = new Date(state.time).toLocaleTimeString();
        return time;
    }
}

register(Clock, 'x-c');

function renderApp() {
    render((<x-c/>), document.body, document.body.firstElementChild);
}

renderApp();
console.log(document.body.innerHTML);
renderApp();
console.log(document.body.innerHTML);

Output is:

<x-c>3:27:10 PM</x-c>
<x-c></x-c>  // bad: should render time as well

This works in this way because when preact is come to patch <x-c> is removing children of it. <x-c> is not a component is regular node for preact, so on second time render preact is removing children of it because vnode do not contains any children, custom-element do not get any livecycle notification so it is not rerender it content again.

In other words custom element should control rendering of it by self, not from outside parent.
But this will require some changes in preact eg:

https://github.com/developit/preact/blob/master/src/vdom/diff.js#L128

should be replaced by:

var isCE = !!window.customElements.get(vnode.nodeName);

if (!isCE) {
    // Optimization: fast-path for elements containing a single TextNode:
    if (!hydrating && vchildren && vchildren.length===1 && typeof vchildren[0]==='string' &&
        fc!=null && fc.splitText!==undefined && fc.nextSibling==null) {
        if (fc.nodeValue!=vchildren[0]) {
            fc.nodeValue = vchildren[0];
        }
    }
    // otherwise, if there are existing or new children, diff them:
    else if (vchildren && vchildren.length || fc!=null) {
        innerDiffNode(out, vchildren, context, mountAll, hydrating ||
            props.dangerouslySetInnerHTML!=null);
    }
} 

Or more generic like https://github.com/Matt-Esch/virtual-dom/blob/master/docs/thunk.md

Webpack.bin: https://www.webpackbin.com/bins/-Ki44_xg5yie0o-jHaXv

majo44 commented 7 years ago

Related issue https://github.com/developit/preact/issues/654

developit commented 7 years ago

I found a way to solve this in your variant of preact-custom-element. Here's the updated bin: https://www.webpackbin.com/bins/-Ki5qi9RpWySLvOGz6H5

bspaulding commented 6 years ago

This appears to still be an issue, although I admittedly don't really understand the issue or the use-case for this.

In any case, I've added a test case for this in the test-rerender-root branch, for anyone who thinks they might have a solution and/or whenever this is solved.

developit commented 4 years ago

This should be working correctly in Preact X. Please re-open if not!