Array.prototype.find icon indicating copy to clipboard operation
Array.prototype.find copied to clipboard

Implementation has a big footprint in JS bundle size

Open thibaudcolas opened this issue 6 years ago • 6 comments

When using this polyfill for the first time I didn't realise it was so big (somewhere between 10 and 20kb depending on how the code is minified). This is in stark contrast with the polyfill displayed on MDN, which is only about a 1000 characters.

I'm sure there are good reasons for this (spec compliance, performance, support for IE 6-8) – it would be nice if the README showcased what exactly those are, and mentioned that there are alternatives for people who do not need the browser support. For example:

  • Approximate file size minified, including dependencies (I can measure that one myself).
  • Browser support (mentioned in https://github.com/paulmillr/Array.prototype.find/issues/23)
  • Spec compliance - could be worth noting that this depends on https://github.com/ljharb/es-abstract?

I'm more than happy to draft a PR for this if there's an agreement it is useful.

thibaudcolas avatar Jun 12 '17 17:06 thibaudcolas

The polyfill on MDN (like most of them) isn't fully spec-compliant across all browsers, which is why these deps exist. Additionally, even modern browsers occasionally have bugs, so every browser needs the full shim, always.

How are you importing it?

I'm not sure how depending on es-abstract matters - the deps are listed in package.json for those that are curious.

Also, the full size of a dep doesn't matter as much as the gzipped, minified size - which is surely far less than 10-20k.

ljharb avatar Jun 12 '17 18:06 ljharb

Here are the real-world numbers with testing command (latest Webpack version), with and without gzipping.

// array-find.js
require('array.prototype.find').shim();
# array.prototype.find polyfill
webpack -p array-find.js array-find.min.js && gzip array-find.min.js && wc -c array-find.min.js.gz
# array-find.min.js  14.4 kB       0  [emitted]  main
#     4439 array-find.min.js.gz

# MDN polyfill
webpack -p mdn.js mdn.min.js && gzip mdn.min.js && wc -c mdn.min.js.gz
# mdn.min.js  847 bytes       0  [emitted]  main
#     465 mdn.min.js.gz

My point is that, being curious and looking at the source as you suggest, I found out why this polyfill is bigger than the one on MDN. It would be interesting to document how it compares to others: spec compliance, file size, performance, fixes browser bugs for you, etc, to help people make an informed decision without looking at the source. I disagree with the "every browser needs the full shim, always", but that's exactly the type of info I'd like to see in the README.

If you think the benefits of this docs change is arguable / too small to justify the effort that's ok, I'll move on and the next person optimising their Webpack bundle may see this issue and find it helpful anyway.

thibaudcolas avatar Jun 12 '17 21:06 thibaudcolas

I think it's fine to enhance the docs; but I don't think the common best practice of maximally correct shims is something that the readme needs to defend; it can simply state it.

ljharb avatar Jun 12 '17 22:06 ljharb

We saw an increase in our bundle caused by chain of events, mostly caused by changes in es-abstract package when we upgraded our node version (and thus npm) from 8.9.4 (5.6) to 12.14.0 (6.13.4)

There's a change in how npm > 5.7 resolves packages and flattens them. It used to be that the sub dependencies would be pinned to fixed versions, but this got changed to a '^' match. This package's spec in package-lock.json went from:

{
	"array.prototype.find": {
		"version": "2.0.4",
		"requires": {
			"define-properties": "1.1.3",
			"es-abstract": "1.13.0"
		}
	}
}

to

{
	"array.prototype.find": {
		"version": "2.0.4",
		"requires": {
			"define-properties": "^1.1.2",
			"es-abstract": "^1.7.0"
		}
	}
}

This resulted in changing the es-abstract version in the node_modules directory, which has a lot of new changes that caused an increase in the bundle size; as much as 9KB gzipped just by this one side-effect. Note that no other change really happened to the version of the find polyfill package. So even if that dependency is listed in the module's package.json, it might end up mattering.

After the nodejs upgrade:

Screenshot 2020-04-15 at 21 56 40

Before the nodejs upgrade: Screenshot 2020-04-15 at 21 56 48

kgrz avatar Apr 15 '20 16:04 kgrz

@kgrz that's because you're on an outdated array.prototype.find. Update all your es-shims polyfills, and they should all end up using the latest es-abstract, and deduping quite efficiently.

ljharb avatar Apr 17 '20 04:04 ljharb

Understood, will check with the latest one. I wasn’t complaining, by the way. It was a surprise when I saw the change.

kgrz avatar Apr 18 '20 09:04 kgrz