preact-compat icon indicating copy to clipboard operation
preact-compat copied to clipboard

WebComponents only working on initial render, not subsequent ones

Open tconroy opened this issue 7 years ago • 13 comments

Hey there, I'm running into some issues getting custom WebComponents (+customElements/natim-shim polyfill) to play nice with preact-compat.

Module versions:

"@webcomponents/custom-elements": "^1.0.0-alpha.3",
"rxjs": "^5.2.0",
"preact": "^7.2.0",
"preact-compat": "3.14.1",
"react": "15.4.2",
"react-dom": "15.4.2",

Environment:

- Node v6.9.1
- NPM v3.10.8
- Webpack 2-based build

the issue: The environment is a SPA. The WebComponent functions as expected on the initial render, but any subsequent renders (IE going from page A to page B), the WebComponent does not render. The tag is present in the markup, and the component is registered, but Preact seems to be preventing the WebComponent from rendering its child nodes.

Similar (solved) issues in the Preact repo:

  • https://github.com/developit/preact/issues/405
  • https://github.com/developit/preact/issues/39

this seems to have been fixed upstream in Preact since v6, however I require preact-compat due to some dependencies. Any fixes or work arounds for this, or any idea what may be causing the issue?

EDIT: also wanted to add, I am using Webpack for the build. The only changes to build I made to incorporate preact was adding the appropriate alias:

{
    // ...
    resolve: {
        alias: {
            'react': 'preact-compat',
            'react-dom': 'preact-compat'
        }
    }
    // ...
}

.. however I notice there are also babel instructions. If you are using Babel in your project, would the babel config be necessary if you are already aliasing in webpack?

tconroy avatar Apr 03 '17 17:04 tconroy

Webpack alias should take care of it. I'm releasing preact 8.0 today, I'll ping here and we should see if that fixes this issue.

developit avatar Apr 05 '17 14:04 developit

Thanks @developit, I'll test it out on my end once 8.0 ships ( congrats on the release! 😄 )

tconroy avatar Apr 05 '17 14:04 tconroy

^ it's out by the way! Let me know if you're still seeing an issue.

developit avatar Apr 18 '17 00:04 developit

hey @developit,

finally got a chance to test with latest version of preact + preact-compat.. unfortunately the issue persists 😢 The first instances render, but subsequent ones (after route change in SPA, etc. ) do not.

For context, these custom elements are for advertisements, each instance loads in a google DFP ad unit. The interface is card-based, and as the user swipes, elements are created/deleted through preact ( including the webcomponent customElements).

Any ideas? Literally the only change I've made is add

    resolve: {
        alias: {
            'react': 'preact-compat',
            'react-dom': 'preact-compat'
        }
    }

to my webpack dist build. If I remove the resolve alias, the components behave as I expect.

tconroy avatar May 02 '17 22:05 tconroy

Hi @tconroy - sorry for the delay there. How are you rendering the child elements? (or binding them to the web components?) Is it a custom element with an attachedCallback that invokes (reactDom/preactCompat).render()?

developit avatar May 14 '17 01:05 developit

Hey @developit! No worries, I'm sure you must be very busy.

So a little overview -- the custom element is wrapped in a react component to make its usage in my react app easier. Something like

class CustomElementWrapper extends Component {
  render() {
    return (
      <my-custom-element a="foo" b="bar" c="baz" />
    );
  }
}

my-custom-element does use an attachedCallback to control its rendering/insertion into the dom, as well as its children. Its children are typically a div and an iframe, which loads some off-domain dynamic content. If you've ever worked with Google DFP, it's a web component to render DFP ad units.

the react-wrapped web component is used in my react app, but never calls (reactDom/preactCompat).render() directly -- that's only done react-side. The custom element only uses RXJS/Observables and native DOM functions.

Sorry I can't really provide a sample page as some of the code is proprietary, but I'm more than happy to answer any questions to help debug this!

tconroy avatar May 14 '17 15:05 tconroy

I wonder if it's being cached.... if so only attachedCallback would be fired, not createdCallback. Thoughts?

developit avatar May 17 '17 06:05 developit

~I do think it has to do with recycling or caching, and my WC's are dependent on the createdCallback being triggered. This sounds like the right trail!~

EDIT: we're using the newer v1 WC spec so I believe the lifecycle methods are a little different, but I do think this has something to do with the node being cached and not destroyed/fully re-created.

tconroy avatar May 17 '17 19:05 tconroy

Hey @developit, our WebComponent is dependent on the following lifecycle hooks (v1 component spec):

  • connectedCallback,
  • attributeChangedCallback
  • disconnectedCallback

we also expect each instance to be a fresh node, and not recycled/cached in any way. They have setup logic we expect to fire. I think that's the issue we're running into, the nodes are not being fully deleted/re-created, but recycled in some way.

tconroy avatar May 24 '17 21:05 tconroy

Totally. Looks like that's directly related to the investigation @Nekr and myself were doing into removing component caching. I agree caching/recycling invalidates assumptions components could be making, particularly when they are DOM components and depend on those lifecycle methods. Only solution here will be to have preact "do less" - remove the caching/recylcing and let the DOM do its thing.

developit avatar May 25 '17 01:05 developit

@developit I'm actually busy with some things last weeks and wasn't looking into the problem since then. But I need to get back it anyway, probably soon.

NekR avatar May 25 '17 02:05 NekR

Thanks for the insight guys! I'm curious @developit will dropping component caching have any negative performance implications for Preact (either now or in the future)?

tconroy avatar May 25 '17 21:05 tconroy

It will impact some silly benchmarks that remount identical components a million times. In the real world though, it will very likely be faster. Will absolutely be better for memory consumption.

developit avatar May 25 '17 22:05 developit