Plugin is polluting Array.prototype

[binury]

This is a really weird one.

Since upgrading from 4.8.3 to 15.* we began to see an interesting browser console error:

Refused to apply style from 'https://localhost/function%20flatten(depth)%20%7B%20%20%20%20var%20o%20=%20Object(this);%20%20%20%20var%20a%20=%20(0,%20_arraySpeciesCreate2.default)(o,%20this.length);%20%20%20%20var%20depthNum%20=%20depth%20!==%20undefined%20?%20Number(depth)%20:%20Infinity;%20%20%20%20(0,%20_flattenIntoArray2.default)(a,%20o,%200,%20depthNum);%20%20%20%20return%20a.filter(function%20(e)%20{%20%20%20%20%20%20return%20e%20!==%20undefined;%20%20%20%20});%20%20}' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

As you can see, somehow a function/method is being inserted into our template as a stylesheet.

The eslint-plugin-jsdoc dependency was not even on the list of suspects. However, we finally observed that downgrading resolved the issue. We can replicate this 100% of the time.

TLDR: Something (some polyfill it seems) in your codebase is modifying Array.prototype with a flatten method that is polluting the webpack plugin namespace.

[binury]

https://github.com/gajus/eslint-plugin-jsdoc/blob/master/src/rules/noUndefinedTypes.js#L2

[binury]

142ac17

[brettz9]

Unless you are opposed to all polyfills, it is not polluting to add a compliant polyfill for a standard or pseudo-standard method like flatMap. I think this is more an issue for webpack to allow exceptions to be made.

[binury]

flat It is erringly looking for flatten: https://github.com/aluanhaddad/flat-map/blob/cce0c60d3b86becfeccf65e60717747bd3bc0cdc/src/flatten.js

The problem arises from the fact that iterating over any array with for...in is going to be caught by this. Obviously we could test with Object.hasOwnProperty but why should this package have say over how our code is written?

[binury]

In our case, this code is inside our main ejs template. At this extent, that most certainly should be called pollution.

[brettz9]

flatten had been renamed at one point, so, yes, it makes sense to avoid that particular method. I've changed the import to import the flatMap import only. As of Node 12, flatMap will be available by default and is not overwritten when found by the polyfill.

[brettz9]

I'll go ahead and close now with the change added to master, though feel free to comment further.

[luislobo]

My main question here is why even include another flatMap function since you already have one in Lodash (that you use)

[binury]

@brettz9 Thanks. This will help

[brettz9]

@luislobo My preference has been to move away from lodash where there is a native equivalent. The semantics are more well-known and understood.

There's nothing wrong with frameworks, of course, but even some features that elegantly save a good deal of code and which we are still using, like lodash's _.get, can be replacable now (or preferably in the future as it starts landing) with close ES equivalents (in this case, the optional chaining operator).

Some of the sugar in lodash may be unexpected for those not familiar with it--e.g., where it may not fail with non-array values, so if there is a native near-equivalent, I'd very much prefer to use that where practical (where there is little difference in the readability/amount of code needed).