Update PolyUtil.js

[davidmgvaz]

Replace for..in to avoid TypeError: Cannot read properties of null (reading 'lat') with extra attributes of array

[jonkoops]

Can you provide an example of how to reproduce this bug?

[davidmgvaz]

I found it in a complicated piece o code, but the principle that caused this bug was the following: // Somewhere deep in your JavaScript library...Array.prototype.foo = 1; // Now you have no idea what the below code will do.var a = [1, 2, 3, 4, 5];for (var x in a){ // Now foo is a part of EVERY array and // will show up here as a value of 'x'. console.log(x); } /* Will display: 0 1 2 3 4 foo */ Jon Koops ***@***.***> escreveu em qui., 9/02/2023 às 18:59 :

Can you provide an example of how to reproduce this bug? — Reply to this email directly, view it on GitHub <#8840 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4IKB4ZKFMGZJXO5RNDKLWWU5BPANCNFSM6AAAAAAUWYZWTA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- David Vaz, PhD

[jonkoops]

@davidmgvaz could you add a regression test for this?

[davidmgvaz]

I am not too comfortable to create that. I just found a common error, and decided to warn about it.

I literally changed 2 lines of code, easy to check by any reviewer. Can you please help?

[Falke-Design]

I added the same fix to LineUtil and also tests.

Another solutions woule be hasOwnProperty

	const points = [];
	for (const k in latlngs) {
		if(latlngs.hasOwnProperty(k)) {
			points.push(crs.project(toLatLng(latlngs[k])));
		}
	}

[davidmgvaz]

Another solutions would be hasOwnProperty

True, and safer for partially filled arrays e.g

const x = [];
x[5] = 0;
for (let i=0; i < x.length; i++) {console.log(x[i]);}

[mourner]

Just wondering, why is it a for ... in loop in the first place, iterating over an array? Looks like a mistake that slipped in earlier. This probably should be a for ... of, right?

[davidmgvaz]

even better. Perfect.

outdated

[jonkoops]

@Falke-Design @mourner a quick enable of the guard-for-in rule brings up a lot of red flags:

Although these are of course not all bugs like this one, they would have the potential to be. Perhaps we should enable this rule and replace all existing loops with for...of, WDYT?

[Falke-Design]

I have in mind that not all usages of for ... in can be replaced with for ... of, but else yeah makes maybe sense

[jonkoops]

I have in mind that not all usages of for ... in can be replaced with for ... of, but else yeah makes maybe sense

Yeah I wonder how many we can replace and which will remain. I've created an issue for it (#8843) with a 'help wanted' label, since it's not really high prio, but should be looked into by someone.