Element#getChild modifies passed array

[Piotrek Koszuliński]

Using shift() on a passed argument is a very bad practice.

Setting milestone to 4.5 because unfortunately fix will affect code using getChild().

[Artur Delura]

Shouldn't we create parallel method which doesn't modify passed argument, and mark existing one as a deprecated? I suppose that there might be various 3rd party plugins, extensions etc. which use this method and are designed to work just with current implementation. I mean that migration can be difficult, and some plugins might stop working.

[Marek Lewandowski]

I think we can't affort to make a duplicates in api.

As for this particular change I can't imagine any reasonable usage of the fact that getChild() consumes your array.

Eventually for some "hc optimization enthusiast" who considers every new array a harmful loss of performance.

eg.

var paragraphPath = [ 0, 0, 1 ],
	paragraphChild;
	(...)
	paragraphChild.getChild( paragraphPath );
	
	// Now paragraphPath path will store customer orders! We don't create an
	// extra var for that, because this is a waste of resources!
	// Use paragraphPath as customer orders!
	app.getCustomerOrders( paragraphPath );
	
	(...)
	app.view.displayOrders( paragraphPath );

I don't think that we have lot of such "hardcore" developers out there :)

Summary:

• I wouldn't expect array parameter to be used. This method is most useful with number.
• there's no reasonable (API) UC that would be broken if we fix the method
• as mentioned, typically programmer will expect his input params not to be modified (if it's not mentioned in docs)

Still lets wait what other our devs have to say about that.

[Artur Delura]

Okay, after some talks we decided that we shouldn't create parallel solution because it's just a bug and it's not documented as well.

[Artur Delura]

Changes and tests in ​branch:t/12148.

[Piotrek Koszuliński]

This branch should be based on major, not master. I'll rebase it.

[Artur Delura]

Replying to Reinmar:

This branch should be based on major, not master. I'll rebase it.

Sorry for that, I get used to master.

[Artur Delura]

I rebased it to major.

[Piotrek Koszuliński]

Fixed on major with ​git:8969f97.