Re: oldNode.replaceWith(...collection) edge case

[Anne van Kesteren]

On Thu, Jan 22, 2015 at 2:18 AM, Glen Huang  wrote:
> This algorithm shouldn’t slow normal operations down, and I wonder if the 
> spec could use an algorithm like this and not using document fragment.

I looked at this again and we do want to use a DocumentFragment.
Otherwise mutation observers get messy. But I guess we could store
offset parent and next sibling either way. I filed

  https://github.com/whatwg/dom/issues/32

to tackle this. These algorithms could use some cleaning up in general.


-- 
https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Jonas Sicking]

On Jan 27, 2015 4:51 AM, "Anne van Kesteren"  wrote:
>
> On Thu, Jan 22, 2015 at 11:43 AM, Jonas Sicking  wrote:
> > In general I agree that it feels unintuitive that you can't replace a
node
> > with a collection which includes the node itself. So the extra line or
two
> > of code seems worth it.
>
> You don't think it's weird that before/after/replaceWith all end up
> doing the same for that scenario? Perhaps it's okay..

Yeah, I think that's okay.

/ Jonas

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

before/after/replaceWith behave the same in this case is just a side effect of 
DOM trying to be less surprising and more symmetrical for the curious ones. I 
doubt most people would even aware they behave the same in this case. Whenever 
the user cases come, I believe most people will just use replaceWith.

> On Jan 27, 2015, at 8:51 PM, Anne van Kesteren  wrote:
> 
> On Thu, Jan 22, 2015 at 11:43 AM, Jonas Sicking  wrote:
>> In general I agree that it feels unintuitive that you can't replace a node
>> with a collection which includes the node itself. So the extra line or two
>> of code seems worth it.
> 
> You don't think it's weird that before/after/replaceWith all end up
> doing the same for that scenario? Perhaps it's okay...
> 
> 
> -- 
> https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Anne van Kesteren]

On Thu, Jan 22, 2015 at 11:43 AM, Jonas Sicking  wrote:
> In general I agree that it feels unintuitive that you can't replace a node
> with a collection which includes the node itself. So the extra line or two
> of code seems worth it.

You don't think it's weird that before/after/replaceWith all end up
doing the same for that scenario? Perhaps it's okay...


-- 
https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Jonas Sicking]

On Jan 17, 2015 8:20 PM, "Glen Huang"  wrote:
>
> Oh crap. Just realized saving index won't work if context node's previous
siblings are passed as arguments. Looks like inserting transient node is
still the best way.

The simplest way to write this method would seem to me to be something like:

Node.prototype.replaceWith = function(collection) {
  if (collection instanceof Node)
collection = [collection];

  var following = this.nextSibling;
  var parent = this.parentNode;
  parent.removeChild(this);
  for (node of collection) {
if (node == following) {
  following = following.nextSibling;
  continue;
}

if (node.nodeType == FRAGMENT) {
  var last = node.lastChild;
}

parent.insertBefore(node, following);

if (node.nodeType == FRAGMENT) {
  following = last.nextSibling;
}
  }
}

In general I agree that it feels unintuitive that you can't replace a node
with a collection which includes the node itself. So the extra line or two
of code seems worth it.

/ Jonas

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

I have two more things to add.

1. One more reason why context node should be allowed as an argument.

These work as intended:

node.before(node)
node.after(node)
node.replaceWith(node)

By passing an additional argument, they suddenly fail:

node.before(node, another)
node.after(node, another)
node.replaceWith(node, another)

This doesn’t feel right.

2. Algorithm performance

The performance hit is based on the assumption that an implementation will 
follow the spec algorithm closely and use a document fragment when multiple 
arguments are passed. In reality, I believe an implementation can totally get 
away from that and insert arguments directly, as long as it can make sure the 
final DOM structure is correct, mutation records are queued, ranges are 
modified, etc. Nothing in the algorithm dictates the using of a document 
fragment.

For example, for the before() method, I can imagine it could be implemented it 
like this (in pseudo code, ignoring mutation records and range, etc):

ensure context node validity
set prevInserted to null
for node from last argument to first argument, if the argument is a document 
fragment, from its last child to first child
if node is the context node
if prevInserted is not null
insert node before it
set prevInserted to node
else
set prevInserted to node
continue
else
insert node before the context node
set prevInserted to node

This algorithm shouldn’t slow normal operations down, and I wonder if the spec 
could use an algorithm like this and not using document fragment.

> On Jan 21, 2015, at 5:50 PM, Glen Huang  wrote:
> 
> @Boris @Simon
> 
> From the jsperf, looks like Blink is indeed making document fragment 
> obsolete. Run the tests in webkit, although still way faster with document 
> fragment, I believe the gap will narrow in the future. (I really think 
> authors shouldn’t have to rely on it to get good performance).
> 
> Thank you for debunking the myth for me. :)
> 
> Now, let’s go back to the original topic. I brought up document fragment 
> because I wanted to argue that by disallowing passing the context node as an 
> argument, authors would be unable find an equally performant solution. You 
> guys tell me that’s not the case. I agree, and I will drop that.
> 
> But I still don’t feel disallowing that is the right way to go, for two 
> reasons:
> 
> 1. Passing context node as an argument does have a meaningful result, and 
> practical use cases.
> 2. Although the use cases might not come as often, these are native DOM 
> methods, everybody is expected to use them. Given the huge user base, the use 
> cases might not come as rare also.
> 
> I see the argument against it is probably that it may slow down normal 
> operations too. But is that really true? The key here is to find the correct 
> insertion point after the macro action. Although in the spec algorithm, using 
> a transient node seems to be the most performant way (and it slows down 
> normal operations), I doubt it’s impossible for an actual implementation to 
> optimize that.
> 
> This isn't some feature that can be disallowed now and allowed in the future. 
> And by disallowing it, I think it qualifies as a gotcha when you do need it.
> 
> But that’s just my personal feelings. If it turns out it’s really not that 
> cheap to implement. I’d be happy to correctly use before() and after() and 
> not root for something that will slow everybody down. :)
> 
>> On Jan 21, 2015, at 4:52 PM, Simon Pieters  wrote:
>> 
>> On Wed, 21 Jan 2015 00:45:32 +0100, Glen Huang  wrote:
>> 
>>> Ah, thank you for letting me know.
>>> 
>>> I vaguely remember document fragment is introduced just to reduce reflows. 
>>> Looks like this best practice is obsolete now? (I remember myself wondering 
>>> why bowsers couldn’t optimize that back then.) Many people still suggest it 
>>> though, including google 
>>> (https://developers.google.com/speed/articles/javascript-dom 
>>>  the 
>>> "DocumentFragment DOM Generation” section), and you can find more by 
>>> googling “why use document fragment".
>> 
>> I think that article is a bit misguided. Changing a class does trigger a 
>> reflow, but it doesn't force a reflow while the script is running (maybe it 
>> does in old browsers). Asking for layout information does force a reflow.
>> 
>> I think documentfragment has been faster in several browsers and maybe still 
>> is, but in Blink at least it appears that the different methods are getting 
>> about equally fast. It probably depends on how you do it, though. This 
>> jsperf might be interesting:
>> 
>> http://jsperf.com/appendchild-vs-documentfragment-vs-innerhtml/81
>> 
>>> So to recap, when you have the need to pass the context node as an argument 
>>> along with other nodes, just use bef

Re: oldNode.replaceWith(...collection) edge case

[Boris Zbarsky]

On 1/21/15 3:52 AM, Simon Pieters wrote:

This jsperf might be interesting:

http://jsperf.com/appendchild-vs-documentfragment-vs-innerhtml/81


Or misleading.

Again, in real life what matters most here is what else is on the page, 
which the jsperf doesn't capture.  The other thing that really matters 
is how many elements are being inserted, exactly, of course... which the 
jsperf also doesn't capture.


Typically, jsperf is the wrong tool for answering questions like this, 
unless you're 100% sure your microbenchmark captures all the essential 
features of the macro-situation...


-Boris

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

@Boris @Simon

From the jsperf, looks like Blink is indeed making document fragment obsolete. 
Run the tests in webkit, although still way faster with document fragment, I 
believe the gap will narrow in the future. (I really think authors shouldn’t 
have to rely on it to get good performance).

Thank you for debunking the myth for me. :)

Now, let’s go back to the original topic. I brought up document fragment 
because I wanted to argue that by disallowing passing the context node as an 
argument, authors would be unable find an equally performant solution. You guys 
tell me that’s not the case. I agree, and I will drop that.

But I still don’t feel disallowing that is the right way to go, for two reasons:

1. Passing context node as an argument does have a meaningful result, and 
practical use cases.
2. Although the use cases might not come as often, these are native DOM 
methods, everybody is expected to use them. Given the huge user base, the use 
cases might not come as rare also.

I see the argument against it is probably that it may slow down normal 
operations too. But is that really true? The key here is to find the correct 
insertion point after the macro action. Although in the spec algorithm, using a 
transient node seems to be the most performant way (and it slows down normal 
operations), I doubt it’s impossible for an actual implementation to optimize 
that.

This isn't some feature that can be disallowed now and allowed in the future. 
And by disallowing it, I think it qualifies as a gotcha when you do need it.

But that’s just my personal feelings. If it turns out it’s really not that 
cheap to implement. I’d be happy to correctly use before() and after() and not 
root for something that will slow everybody down. :)

> On Jan 21, 2015, at 4:52 PM, Simon Pieters  wrote:
> 
> On Wed, 21 Jan 2015 00:45:32 +0100, Glen Huang  wrote:
> 
>> Ah, thank you for letting me know.
>> 
>> I vaguely remember document fragment is introduced just to reduce reflows. 
>> Looks like this best practice is obsolete now? (I remember myself wondering 
>> why bowsers couldn’t optimize that back then.) Many people still suggest it 
>> though, including google 
>> (https://developers.google.com/speed/articles/javascript-dom 
>>  the 
>> "DocumentFragment DOM Generation” section), and you can find more by 
>> googling “why use document fragment".
> 
> I think that article is a bit misguided. Changing a class does trigger a 
> reflow, but it doesn't force a reflow while the script is running (maybe it 
> does in old browsers). Asking for layout information does force a reflow.
> 
> I think documentfragment has been faster in several browsers and maybe still 
> is, but in Blink at least it appears that the different methods are getting 
> about equally fast. It probably depends on how you do it, though. This jsperf 
> might be interesting:
> 
> http://jsperf.com/appendchild-vs-documentfragment-vs-innerhtml/81
> 
>> So to recap, when you have the need to pass the context node as an argument 
>> along with other nodes, just use before() and after() to insert these other 
>> nodes? And even insert them one by one is fine?
> 
> Yeah.
> 
> -- 
> Simon Pieters
> Opera Software

Re: oldNode.replaceWith(...collection) edge case

[Simon Pieters]

On Wed, 21 Jan 2015 00:45:32 +0100, Glen Huang   
wrote:



Ah, thank you for letting me know.

I vaguely remember document fragment is introduced just to reduce  
reflows. Looks like this best practice is obsolete now? (I remember  
myself wondering why bowsers couldn’t optimize that back then.) Many  
people still suggest it though, including google  
(https://developers.google.com/speed/articles/javascript-dom  
 the  
"DocumentFragment DOM Generation” section), and you can find more by  
googling “why use document fragment".


I think that article is a bit misguided. Changing a class does trigger a  
reflow, but it doesn't force a reflow while the script is running (maybe  
it does in old browsers). Asking for layout information does force a  
reflow.


I think documentfragment has been faster in several browsers and maybe  
still is, but in Blink at least it appears that the different methods are  
getting about equally fast. It probably depends on how you do it, though.  
This jsperf might be interesting:


http://jsperf.com/appendchild-vs-documentfragment-vs-innerhtml/81

So to recap, when you have the need to pass the context node as an  
argument along with other nodes, just use before() and after() to insert  
these other nodes? And even insert them one by one is fine?


Yeah.

--
Simon Pieters
Opera Software

Re: oldNode.replaceWith(...collection) edge case

[Boris Zbarsky]

On 1/20/15 6:45 PM, Glen Huang wrote:

I vaguely remember document fragment is introduced just to reduce
reflows. Looks like this best practice is obsolete now?


You don't have to use a document fragment to reduce reflows, as long as 
you don't query layout information between your DOM notifications.


There are some other things (e.g. maintaining DOM state of various 
sorts) a browser may be able to optimize a bit better with a document 
fragment.  Or not.



So to recap, when you have the need to pass the context node as an
argument along with other nodes, just use before() and after() to insert
these other nodes? And even insert them one by one is fine?


I strongly suggest measuring to get an answer to this question.  The 
performance characteristics will depend on the browser, on the exact set 
of DOM features used on the page, and on the exact stylesheets used on 
the page.


That said, I would expect the difference between inserting a document 
fragment and inserting the nodes one at a time to be fairly small in all 
modern browsers in most situations.  Note all the weasel-wording.  ;)


-Boris

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

Ah, thank you for letting me know.

I vaguely remember document fragment is introduced just to reduce reflows. 
Looks like this best practice is obsolete now? (I remember myself wondering why 
bowsers couldn’t optimize that back then.) Many people still suggest it though, 
including google (https://developers.google.com/speed/articles/javascript-dom 
 the 
"DocumentFragment DOM Generation” section), and you can find more by googling 
“why use document fragment".

So to recap, when you have the need to pass the context node as an argument 
along with other nodes, just use before() and after() to insert these other 
nodes? And even insert them one by one is fine?

> On Jan 20, 2015, at 11:57 PM, Simon Pieters  wrote:
> 
> On Tue, 20 Jan 2015 15:00:41 +0100, Glen Huang  wrote:
> 
>> I wonder what the correct method should be? For the example I gave in the 
>> previous mail, it looks like I have to either create two fragments (and 
>> compute which nodes go to which fragment) and insert them before or after 
>> the node (two reflows), or implement the transient node algorithm myself 
>> (but with no suppressing observer ability, also three reflows (insert fake 
>> node, pull out context node, insert fragment), i guess if browsers implement 
>> it natively, they can reduce it to just one reflow?). Both doesn’t sound 
>> very optimal.
> 
> In all cases it would be just one reflow after the script has finished, 
> unless you force a reflow by asking for layout information (e.g. offsetTop) 
> between the mutations.
> 
> -- 
> Simon Pieters
> Opera Software

Re: oldNode.replaceWith(...collection) edge case

[Simon Pieters]

On Tue, 20 Jan 2015 15:00:41 +0100, Glen Huang   
wrote:


I wonder what the correct method should be? For the example I gave in  
the previous mail, it looks like I have to either create two fragments  
(and compute which nodes go to which fragment) and insert them before or  
after the node (two reflows), or implement the transient node algorithm  
myself (but with no suppressing observer ability, also three reflows  
(insert fake node, pull out context node, insert fragment), i guess if  
browsers implement it natively, they can reduce it to just one reflow?).  
Both doesn’t sound very optimal.


In all cases it would be just one reflow after the script has finished,  
unless you force a reflow by asking for layout information (e.g.  
offsetTop) between the mutations.


--
Simon Pieters
Opera Software

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

I wonder what the correct method should be? For the example I gave in the 
previous mail, it looks like I have to either create two fragments (and compute 
which nodes go to which fragment) and insert them before or after the node (two 
reflows), or implement the transient node algorithm myself (but with no 
suppressing observer ability, also three reflows (insert fake node, pull out 
context node, insert fragment), i guess if browsers implement it natively, they 
can reduce it to just one reflow?). Both doesn’t sound very optimal.

> On Jan 20, 2015, at 9:34 PM, Anne van Kesteren  wrote:
> 
> On Tue, Jan 20, 2015 at 2:22 PM, Glen Huang  wrote:
>> jQuery doesn’t support that out of performance and code size reasons:
>> 
>> http://bugs.jquery.com/ticket/14380
>> https://github.com/jquery/jquery/pull/1276#issuecomment-24526014
>> 
>> Both reasons shouldn’t be a problem with the native DOM.
> 
> They are. I also realized that if we did this we would have to make
> before(), after(), and replaceWith() identical when passed the context
> object. Not allowing the context object and requiring usage of the
> correct method seems simpler.
> 
> 
> -- 
> https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Anne van Kesteren]

On Tue, Jan 20, 2015 at 2:22 PM, Glen Huang  wrote:
> jQuery doesn’t support that out of performance and code size reasons:
>
> http://bugs.jquery.com/ticket/14380
> https://github.com/jquery/jquery/pull/1276#issuecomment-24526014
>
> Both reasons shouldn’t be a problem with the native DOM.

They are. I also realized that if we did this we would have to make
before(), after(), and replaceWith() identical when passed the context
object. Not allowing the context object and requiring usage of the
correct method seems simpler.


-- 
https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

jQuery doesn’t support that out of performance and code size reasons: 

http://bugs.jquery.com/ticket/14380 
https://github.com/jquery/jquery/pull/1276#issuecomment-24526014 


Both reasons shouldn’t be a problem with the native DOM.

> On Jan 20, 2015, at 6:39 PM, Anne van Kesteren  wrote:
> 
> On Sun, Jan 18, 2015 at 4:40 AM, Glen Huang  wrote:
>> To generalize the use case, when you have a bunch of nodes, some of which 
>> need to be inserted before a node, and some of which after it, you are 
>> likely to want `replaceWith` could accept the context node as an argument.
> 
> This sound somewhat reasonable but I haven't been able to reproduce
> this in existing libraries. E.g. in Jquery
> 
>  $("div").replaceWith([$("div"), "test"])
> 
> ends up as just test...
> 
> 
> -- 
> https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Anne van Kesteren]

On Sun, Jan 18, 2015 at 4:40 AM, Glen Huang  wrote:
> To generalize the use case, when you have a bunch of nodes, some of which 
> need to be inserted before a node, and some of which after it, you are likely 
> to want `replaceWith` could accept the context node as an argument.

This sound somewhat reasonable but I haven't been able to reproduce
this in existing libraries. E.g. in Jquery

  $("div").replaceWith([$("div"), "test"])

ends up as just test...


-- 
https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

Oh crap. Just realized saving index won’t work if context node’s previous 
siblings are passed as arguments. Looks like inserting transient node is still 
the best way.

> On Jan 18, 2015, at 11:40 AM, Glen Huang  wrote:
> 
> To generalize the use case, when you have a bunch of nodes, some of which 
> need to be inserted before a node, and some of which after it, you are likely 
> to want `replaceWith` could accept the context node as an argument.
> 
> I just realized another algorithm: Before running the macro, save the context 
> node’s index and its parent, and after running it, pre-insert node into 
> parent before parent’s index’th child (could be null). No transient node 
> involved and no recursive finding.
> 
> Hope you can reconsider if this edge case should be accepted.
> 
>> On Jan 16, 2015, at 5:04 PM, Glen Huang  wrote:
>> 
>> Oh, right. Trying to be smart and it just proved otherwise. :P
>> 
>>> I don't really see a good reason to complicate the algorithm for this 
>>> scenario, personally.
>> 
>> This edge case may seem absurd at first sight. Let me provide a use case:
>> 
>> Imagine you have this simple site
>> 
>> ```
>> 
>>  Blog
>>  About
>>  Contact
>> 
>> About page content
>> ```
>> 
>> You are currently at the about page. What you are trying to do is that when 
>> the user clicks a nav link, the corresponding page is fetched via ajax, and 
>> inserted before or after the current main element, depending on whether the 
>> clicked nav link exists before or after the current nav link.
>> 
>> So when the page is first loaded, you first loop over the nav links to 
>> create empty mains for placeholder purposes.
>> 
>> ```
>> 
>>  Blog
>>  About
>>  Contact
>> 
>> 
>> About page content
>> 
>> ```
>> 
>> How do you do that? Well, ideally, you should be able to just do (in pseudo 
>> code):
>> 
>> ```
>> currentMain = get the main element
>> links = get all a elements
>> mains = []
>> 
>> for i, link in links
>>  if link is current link
>>  mains[i] = currentMain
>>  else
>>  mains[i] = clone currentMain shallowly
>> 
>> currentMain.replaceWith(…mains)
>> ```
>> 
>> This way you are inserting nodes in batch, and not having to deal with 
>> choosing insertBefore or appendChild.
>> 
>> Without `replaceWith` supporting it, in order to do batch insertions (nav 
>> links could be a large list, imagining a very long TOC links), you are 
>> forced to manually do the steps I mentioned in the first mail.
>> 
>>> On Jan 16, 2015, at 4:22 PM, Anne van Kesteren  wrote:
>>> 
>>> On Fri, Jan 16, 2015 at 8:47 AM, Glen Huang  wrote:
 Another way to do this is that in mutation method macro, prevent `oldNode` 
 from being added to the doc frag, and after that, insert the doc frag 
 before `oldNode`, finally remove `oldNode`. No recursive finding of next 
 sibling is needed this way.
>>> 
>>> But then d2 would no longer be present?
>>> 
>>> I don't really see a good reason to complicate the algorithm for this
>>> scenario, personally.
>>> 
>>> 
>>> -- 
>>> https://annevankesteren.nl/
>> 
> 

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

To generalize the use case, when you have a bunch of nodes, some of which need 
to be inserted before a node, and some of which after it, you are likely to 
want `replaceWith` could accept the context node as an argument.

I just realized another algorithm: Before running the macro, save the context 
node’s index and its parent, and after running it, pre-insert node into parent 
before parent’s index’th child (could be null). No transient node involved and 
no recursive finding.

Hope you can reconsider if this edge case should be accepted.

> On Jan 16, 2015, at 5:04 PM, Glen Huang  wrote:
> 
> Oh, right. Trying to be smart and it just proved otherwise. :P
> 
>> I don't really see a good reason to complicate the algorithm for this 
>> scenario, personally.
> 
> This edge case may seem absurd at first sight. Let me provide a use case:
> 
> Imagine you have this simple site
> 
> ```
> 
>   Blog
>   About
>   Contact
> 
> About page content
> ```
> 
> You are currently at the about page. What you are trying to do is that when 
> the user clicks a nav link, the corresponding page is fetched via ajax, and 
> inserted before or after the current main element, depending on whether the 
> clicked nav link exists before or after the current nav link.
> 
> So when the page is first loaded, you first loop over the nav links to create 
> empty mains for placeholder purposes.
> 
> ```
> 
>   Blog
>   About
>   Contact
> 
> 
> About page content
> 
> ```
> 
> How do you do that? Well, ideally, you should be able to just do (in pseudo 
> code):
> 
> ```
> currentMain = get the main element
> links = get all a elements
> mains = []
> 
> for i, link in links
>   if link is current link
>   mains[i] = currentMain
>   else
>   mains[i] = clone currentMain shallowly
> 
> currentMain.replaceWith(…mains)
> ```
> 
> This way you are inserting nodes in batch, and not having to deal with 
> choosing insertBefore or appendChild.
> 
> Without `replaceWith` supporting it, in order to do batch insertions (nav 
> links could be a large list, imagining a very long TOC links), you are forced 
> to manually do the steps I mentioned in the first mail.
> 
>> On Jan 16, 2015, at 4:22 PM, Anne van Kesteren  wrote:
>> 
>> On Fri, Jan 16, 2015 at 8:47 AM, Glen Huang  wrote:
>>> Another way to do this is that in mutation method macro, prevent `oldNode` 
>>> from being added to the doc frag, and after that, insert the doc frag 
>>> before `oldNode`, finally remove `oldNode`. No recursive finding of next 
>>> sibling is needed this way.
>> 
>> But then d2 would no longer be present?
>> 
>> I don't really see a good reason to complicate the algorithm for this
>> scenario, personally.
>> 
>> 
>> -- 
>> https://annevankesteren.nl/
> 

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

Here is another try:

How about before executing the mutation method macro, we insert a transient 
node after `oldNode`, suppressing observers. Then run the mutation method 
macro, pre-insert `node` before the transient node and finally remove the 
transient node, suppressing observers.

Idea comes from Andrea’s DOM4 library: 
https://github.com/WebReflection/dom4/commit/ffc8cbdf88fa98627dd82cf11084a0660b9bbfc0
 


> On Jan 16, 2015, at 4:22 PM, Anne van Kesteren  wrote:
> 
> On Fri, Jan 16, 2015 at 8:47 AM, Glen Huang  wrote:
>> Another way to do this is that in mutation method macro, prevent `oldNode` 
>> from being added to the doc frag, and after that, insert the doc frag before 
>> `oldNode`, finally remove `oldNode`. No recursive finding of next sibling is 
>> needed this way.
> 
> But then d2 would no longer be present?
> 
> I don't really see a good reason to complicate the algorithm for this
> scenario, personally.
> 
> 
> -- 
> https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

Oh, right. Trying to be smart and it just proved otherwise. :P

> I don't really see a good reason to complicate the algorithm for this 
> scenario, personally.

This edge case may seem absurd at first sight. Let me provide a use case:

Imagine you have this simple site

```

Blog
About
Contact

About page content
```

You are currently at the about page. What you are trying to do is that when the 
user clicks a nav link, the corresponding page is fetched via ajax, and 
inserted before or after the current main element, depending on whether the 
clicked nav link exists before or after the current nav link.

So when the page is first loaded, you first loop over the nav links to create 
empty mains for placeholder purposes.

```

Blog
About
Contact


About page content

```

How do you do that? Well, ideally, you should be able to just do (in pseudo 
code):

```
currentMain = get the main element
links = get all a elements
mains = []

for i, link in links
if link is current link
mains[i] = currentMain
else
mains[i] = clone currentMain shallowly

currentMain.replaceWith(…mains)
```

This way you are inserting nodes in batch, and not having to deal with choosing 
insertBefore or appendChild.

Without `replaceWith` supporting it, in order to do batch insertions (nav links 
could be a large list, imagining a very long TOC links), you are forced to 
manually do the steps I mentioned in the first mail.

> On Jan 16, 2015, at 4:22 PM, Anne van Kesteren  wrote:
> 
> On Fri, Jan 16, 2015 at 8:47 AM, Glen Huang  wrote:
>> Another way to do this is that in mutation method macro, prevent `oldNode` 
>> from being added to the doc frag, and after that, insert the doc frag before 
>> `oldNode`, finally remove `oldNode`. No recursive finding of next sibling is 
>> needed this way.
> 
> But then d2 would no longer be present?
> 
> I don't really see a good reason to complicate the algorithm for this
> scenario, personally.
> 
> 
> -- 
> https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Anne van Kesteren]

On Fri, Jan 16, 2015 at 8:47 AM, Glen Huang  wrote:
> Another way to do this is that in mutation method macro, prevent `oldNode` 
> from being added to the doc frag, and after that, insert the doc frag before 
> `oldNode`, finally remove `oldNode`. No recursive finding of next sibling is 
> needed this way.

But then d2 would no longer be present?

I don't really see a good reason to complicate the algorithm for this
scenario, personally.


-- 
https://annevankesteren.nl/

Re: oldNode.replaceWith(...collection) edge case

[Glen Huang]

Another way to do this is that in mutation method macro, prevent `oldNode` from 
being added to the doc frag, and after that, insert the doc frag before 
`oldNode`, finally remove `oldNode`. No recursive finding of next sibling is 
needed this way.

> On Jan 16, 2015, at 1:37 PM, Glen Huang  wrote:
> 
> Currently, for `oldNode.replaceWith(…collection)`, if `collection` is array 
> of multiple nodes, and `oldNode` is in `collection`, after the mutation 
> method macro,  `oldNode` lives in a doc frag. So in the replace algorithm, 
> `parent` is the doc frag, `node` is also the doc frag, an 
> `HierarchyRequestError` is thrown.
> 
> I wonder if an error really should be thrown in this case? Intuitively, 
> `collection` should be inserted before `oldNode`’s original next sibling.
> 
> For example:
> 
> ```
> 
> 
> 
> 
> ```
> 
> Imagine `oldNode` is #d2, `collection` is [#d1,#d2,#d4], executing 
> `oldNode.replaceWith(…collection)` should give
> 
> ```
> 
> 
> 
> 
> ```
> 
> Instead of throwing an error.
> 
> To make it this work, before executing the mutation method macro, `oldNode`’s 
> parent should be saved. It’s next sibling should also be saved, but the next 
> sibling need to be found recursively if it happens to be in `collection` too.
> 
> So, If I’m not wrong, this edge case could work in principle. I’m not sure if 
> there is any interest to allow this?