Closed TastyPi closed 1 year ago
@seanpdoyle who introduced the Render
type in #431
It looks like it should be defined (currentElement, newElement)
https://github.com/hotwired/turbo/blob/0f3632a2fe1e6118f8c0e8870e59ace91445e282/src/core/drive/page_renderer.ts#L161
@TastyPi thank you for opening this issue.
Yes, the order of the arguments in the Render
signature are incorrect, and were introduced in that incorrect order:
Luckily, the default implementations and the tests that exercise that feature are in the correct order.
The signature's argument order was inspired by morphdom's, with the latter argument serving as overrides for the prior argument.
Fortunately (and unfortunately!), custom rendering isn't yet covered by Turbo's documentation website, so we have an opportunity to document it thoroughly and correctly.
In the meantime, I'll open a pull request to swap the order of the arguments for TypeScript's sake.
I've opened https://github.com/hotwired/turbo/pull/814 to resolve this.
Some documentation would be great, I've just found that my custom rendering logic breaks back/forward navigation and I have no idea why.
I'm not sure if I'm just misunderstanding something or if they are actually the wrong way round, but it took me a while to figure out what the issue was so this at least might help someone figure it out. The Render type is defined as
(newElement: E, currentElement: E) => void
, but it seems like it should be(currentElement: E, newElement: E) => void
. Let me explain.For background: We use Front's live chat widget, and their library adds an iframe to the body than can never be reloaded. This means we need to keep the same body element as well. The original workaround we tried used morphdom, but then we ran into this issue of Stimulus controllers not reloading. So we decided to implement a custom renderer that keeps the body and iframe but copies everything else across.
This is the first attempt:
This doesn't work, it looks like it just removes all the children without copying the new ones across. I assumed the
newElement
was the newly rendered body and thecurrentElement
was the body currently in the DOM. Turns out they're the other way round, and swapping the parameter order made it start working - it looked like it was just removing elements because the elements were being moved the wrong way.The naming of these parameters is very confusing to me, how could the existing DOM be considered the
newElement
? Hence this issue, are the parameters named the wrong way round or am I misunderstanding the meaning of the termsnew
andcurrent
?