preact.render third argument causes weird behavior

Created on 26 Aug 2019  路  21Comments  路  Source: preactjs/preact

https://codepen.io/laino/pen/XWrMyda?editors=0010 Edit: fixed link.

When using the third "replace node" argument, you won't get anything at all.

Comment either of those render and it will work.

bug

Most helpful comment

Please do not use passive-aggressive language

I can do that. In return please try to take your time to actually understand the reported problem before giving an example that has nothing to do with it and is a workaround at best. That was absolutely infuriating.

@laino Please re-read our Code of Conduct again. We don't tolerate passive aggressiveness as it's in direct violation of our CoC, so we have to flag this. There is never a need to be rude even if commenters may be talking past each other. A good step forward would be to apologize to @JoviDeCroock and than let's get back onto solving this issue :+1:

There does indeed seem to be an issue lurking with the replaceNode parameter. Not rendering twice is a valid workaround in the short term, but that's obviously not a long-term solution.

All 21 comments

@laino The render function has changed.
https://github.com/marvinhagemeister/preact-faq/blob/master/migrate-to-x.md#remove-the-3rd-argument-to-render

@hectorromo The TypeScript types don't reflect that though and clearly the third argument does something.

@JoviDeCroock It doesn't work though. As shown in that codepen. It just causes nothing to be added at all.

Wait.... I linked the wrong codepen.

This thirth argument does not work for the /compat only for the normal Preact distribution

@JoviDeCroock It's not using compat? At least not unless there's some magic happening I'm not aware of.

I've made a small example to show it works since I can't edit on codepen: https://codesandbox.io/s/pensive-mahavira-jgh4h

It's similar to how the CodePen seems to work you're trying to replace an innerhtml

@JoviDeCroock Your example doesn't use PreactX

@JoviDeCroock I fixed it to demonstrate the broken behavior:
https://codesandbox.io/s/practical-glade-kdcip

You'd expect to see "3" there, but instead you get an empty element.

Well again, you are double rendering. I don't see the point in that let's maybe look at what use case that is used for?

It even happens when you render something different. It doesn't have to be the same element.

I even had it set up like that in my codepen, but then you came up with an example that had nothing to do with the problem.

I feel like I'm talking with a wall here.

@JoviDeCroock There, now I'm trying to render something different and have it replace the result of my first render: https://codesandbox.io/s/cool-fog-zl4jj

Except... you get nothing.

Please do not use passive-aggressive language, I'm also just doing this to help out I dedicate my free time in a holiday to answer your question.
I've pointed out the issue in saying that you're using render twice this is probably the root cause of your issue.

Then I showed you an example which has working output, it replaces the existing DOM with correct diffed DOM. This comes in handy when using prerendering and related.

https://codesandbox.io/s/vigilant-kare-x9lbc

Please do not use passive-aggressive language

I can do that. In return please try to take your time to actually understand the reported problem before giving an example that has nothing to do with it and is a workaround at best. That was absolutely infuriating.

Again: The problem is that the replaceNode doesn't actually work. Meaning: Not as documented and probably not at all.

It's supposed to either replace the specified node or update it. Instead you get an empty element:

https://codesandbox.io/s/cool-fog-zl4jj

Please do not use passive-aggressive language

I can do that. In return please try to take your time to actually understand the reported problem before giving an example that has nothing to do with it and is a workaround at best. That was absolutely infuriating.

@laino Please re-read our Code of Conduct again. We don't tolerate passive aggressiveness as it's in direct violation of our CoC, so we have to flag this. There is never a need to be rude even if commenters may be talking past each other. A good step forward would be to apologize to @JoviDeCroock and than let's get back onto solving this issue :+1:

There does indeed seem to be an issue lurking with the replaceNode parameter. Not rendering twice is a valid workaround in the short term, but that's obviously not a long-term solution.

A good step forward would be to apologize to @JoviDeCroock

I am sorry for my choice of metaphorical language.

and than let's get back onto solving this issue +1

Yes, please.

Thanks for defusing, folks. Just an update from me - I've been investigating this off and on and still haven't quite gotten to the bottom of it. However, here's what I have found:

  • The bug is triggered by non-null replaceNode argument resulting in excessDomChildren being populated when entering the diff from render().
  • This is triggering a "sortof hydration" effect, since the existence of excessDomChildren causes each inner diff invocation to also collect and use excessDomChildren as it descends through the tree.
  • Eventually, this causes the Text node to be present in excessDomChildren, and for some reason diffChildren isn't evicting it which causes it to be a removal candidate.
  • My guess (this is just a hunch) is this condition may be triggering the behaviour, since it _uses_ the first entry in excessDomChildren but does not evict it.

One thing to note that may not be quite intuitive to folks looking at the demo: CodeSandbox's console values are reported asynchronously. This is why the <p></p> appears empty when logged between renders. It is in fact <p>3</p> after the first render, and the second render with replaceNode provided causes Text node removal.

Hey @laino, @JoviDeCroock did a fix for your reported issue. The test case is your exactly what you've reported so I'm confident this is fixed.

If there's anything else and this is not working for your needs let us know. Closing it now.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

paulkatich picture paulkatich  路  3Comments

SabirAmeen picture SabirAmeen  路  3Comments

philipwalton picture philipwalton  路  3Comments

marcosguti picture marcosguti  路  3Comments

jasongerbes picture jasongerbes  路  3Comments