-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bundle size query (and 8kB version) #712
Comments
Thanks for flagging this and digging into this! We've had a round of updating our packages and Babel configuration as part of matching the browsers we declared to support and more generally keeping our tools up to date. This changed the polyfills we're including (from core-js 2 to core-js 3, as well as an update of our .browserslistrc), so I'd wager that's where the extra burden comes from. Looks like we need a deeper look at what's being brought in and which options we have for reducing included polyfills, as ideally we'd want only the polyfill code that we'd actively be using and not that for edge cases we'd never encounter. There's still a chance some or all of these polyfills may remain needed, as being compatible with the browsers we want to be compatible with requires both to:
|
Yeah, I understand the reasons - hopefully I can do your deeper look for you here. The current min JS is 54868 bytes. If I comment out I find it funny that https://babeljs.io/docs/options#targets states "We recommend setting targets to reduce the output code size" when I have only ever found it do the opposite. I see this sort of thing has been raised in core-js at e.g. zloirock/core-js#388 or on Reddit at https://www.reddit.com/r/javascript/comments/n5eqie/askjs_is_it_just_me_or_is_corejs_fundamentally/ - core-js always includes maximal IE8- support, even though you've said you don't support that. v4 of core-js will apparently drop that, so that might make a difference, but I still don't think it's at all necessary here. As one example, your code uses Array.prototype.join, which has been in browsers for ever. Because of this, the following dependency tree is added to the distribution: madge dependency tree on es.array.join.jses.array.join.js ../internals/array-method-is-strict.js ../internals/export.js ../internals/function-uncurry-this.js ../internals/indexed-object.js ../internals/to-indexed-object.js ../internals/a-callable.js ../internals/is-callable.js ../internals/try-to-string.js ../internals/an-object.js ../internals/is-object.js ../internals/array-includes.js ../internals/length-of-array-like.js ../internals/to-absolute-index.js ../internals/to-indexed-object.js ../internals/array-method-is-strict.js ../internals/fails.js ../internals/classof-raw.js ../internals/function-uncurry-this.js ../internals/copy-constructor-properties.js ../internals/has-own-property.js ../internals/object-define-property.js ../internals/object-get-own-property-descriptor.js ../internals/own-keys.js ../internals/create-non-enumerable-property.js ../internals/create-property-descriptor.js ../internals/descriptors.js ../internals/object-define-property.js ../internals/create-property-descriptor.js ../internals/define-built-in.js ../internals/define-global-property.js ../internals/is-callable.js ../internals/make-built-in.js ../internals/object-define-property.js ../internals/define-global-property.js ../internals/global.js ../internals/descriptors.js ../internals/fails.js ../internals/document-create-element.js ../internals/global.js ../internals/is-object.js ../internals/engine-user-agent.js ../internals/engine-v8-version.js ../internals/engine-user-agent.js ../internals/global.js ../internals/enum-bug-keys.js ../internals/export.js ../internals/copy-constructor-properties.js ../internals/create-non-enumerable-property.js ../internals/define-built-in.js ../internals/define-global-property.js ../internals/global.js ../internals/is-forced.js ../internals/object-get-own-property-descriptor.js ../internals/fails.js ../internals/function-bind-native.js ../internals/fails.js ../internals/function-call.js ../internals/function-bind-native.js ../internals/function-name.js ../internals/descriptors.js ../internals/has-own-property.js ../internals/function-uncurry-this.js ../internals/function-bind-native.js ../internals/get-built-in.js ../internals/global.js ../internals/is-callable.js ../internals/get-method.js ../internals/a-callable.js ../internals/is-null-or-undefined.js ../internals/global.js ../internals/has-own-property.js ../internals/function-uncurry-this.js ../internals/to-object.js ../internals/hidden-keys.js ../internals/ie8-dom-define.js ../internals/descriptors.js ../internals/document-create-element.js ../internals/fails.js ../internals/indexed-object.js ../internals/classof-raw.js ../internals/fails.js ../internals/function-uncurry-this.js ../internals/inspect-source.js ../internals/function-uncurry-this.js ../internals/is-callable.js ../internals/shared-store.js ../internals/internal-state.js ../internals/create-non-enumerable-property.js ../internals/global.js ../internals/has-own-property.js ../internals/hidden-keys.js ../internals/is-object.js ../internals/shared-key.js ../internals/shared-store.js ../internals/weak-map-basic-detection.js ../internals/is-callable.js ../internals/is-forced.js ../internals/fails.js ../internals/is-callable.js ../internals/is-null-or-undefined.js ../internals/is-object.js ../internals/is-callable.js ../internals/is-pure.js ../internals/is-symbol.js ../internals/get-built-in.js ../internals/is-callable.js ../internals/object-is-prototype-of.js ../internals/use-symbol-as-uid.js ../internals/length-of-array-like.js ../internals/to-length.js ../internals/make-built-in.js ../internals/descriptors.js ../internals/fails.js ../internals/function-name.js ../internals/function-uncurry-this.js ../internals/has-own-property.js ../internals/inspect-source.js ../internals/internal-state.js ../internals/is-callable.js ../internals/math-trunc.js ../internals/object-define-property.js ../internals/an-object.js ../internals/descriptors.js ../internals/ie8-dom-define.js ../internals/to-property-key.js ../internals/v8-prototype-define-bug.js ../internals/object-get-own-property-descriptor.js ../internals/create-property-descriptor.js ../internals/descriptors.js ../internals/function-call.js ../internals/has-own-property.js ../internals/ie8-dom-define.js ../internals/object-property-is-enumerable.js ../internals/to-indexed-object.js ../internals/to-property-key.js ../internals/object-get-own-property-names.js ../internals/enum-bug-keys.js ../internals/object-keys-internal.js ../internals/object-get-own-property-symbols.js ../internals/object-is-prototype-of.js ../internals/function-uncurry-this.js ../internals/object-keys-internal.js ../internals/array-includes.js ../internals/function-uncurry-this.js ../internals/has-own-property.js ../internals/hidden-keys.js ../internals/to-indexed-object.js ../internals/object-property-is-enumerable.js ../internals/ordinary-to-primitive.js ../internals/function-call.js ../internals/is-callable.js ../internals/is-object.js ../internals/own-keys.js ../internals/an-object.js ../internals/function-uncurry-this.js ../internals/get-built-in.js ../internals/object-get-own-property-names.js ../internals/object-get-own-property-symbols.js ../internals/require-object-coercible.js ../internals/is-null-or-undefined.js ../internals/shared-key.js ../internals/shared.js ../internals/uid.js ../internals/shared-store.js ../internals/define-global-property.js ../internals/global.js ../internals/is-pure.js ../internals/shared.js ../internals/shared-store.js ../internals/symbol-constructor-detection.js ../internals/engine-v8-version.js ../internals/fails.js ../internals/global.js ../internals/to-absolute-index.js ../internals/to-integer-or-infinity.js ../internals/to-indexed-object.js ../internals/indexed-object.js ../internals/require-object-coercible.js ../internals/to-integer-or-infinity.js ../internals/math-trunc.js ../internals/to-length.js ../internals/to-integer-or-infinity.js ../internals/to-object.js ../internals/require-object-coercible.js ../internals/to-primitive.js ../internals/function-call.js ../internals/get-method.js ../internals/is-object.js ../internals/is-symbol.js ../internals/ordinary-to-primitive.js ../internals/well-known-symbol.js ../internals/to-property-key.js ../internals/is-symbol.js ../internals/to-primitive.js ../internals/try-to-string.js ../internals/uid.js ../internals/function-uncurry-this.js ../internals/use-symbol-as-uid.js ../internals/symbol-constructor-detection.js ../internals/v8-prototype-define-bug.js ../internals/descriptors.js ../internals/fails.js ../internals/weak-map-basic-detection.js ../internals/global.js ../internals/is-callable.js ../internals/well-known-symbol.js ../internals/global.js ../internals/has-own-property.js ../internals/shared.js ../internals/symbol-constructor-detection.js ../internals/uid.js ../internals/use-symbol-as-uid.js Hope that's helpful. |
Lots of Babel + core-js polyfills are included not due to support but for compatibility But the biggest problem might be Maybe my previous notes in alphagov/govuk-frontend#4557 would be helpful? Some of the findings explained why polyfills for already-supported features were added:
|
Even for compatibility, I don't think any of the extra things being included matter to this library, none of them is fixing any actual compatibility issue with the use of this library as far as I can see, and going further, I personally don't think it should even be up to a library to deal with compatibility with a multitude of apparent core "bugs" - otherwise you end up with every library you use being much larger than it needs to be. Why does it matter if |
@dracos yeah I agree, instead it would be better if the library specified browser compatibility in the README and let end-users polyfill what they need |
Thank you for continuing to maintain this repository. Following on from the similar #321, it looks like v3 increases the size of this package from 11.3kB minified+gzipped (or 8kB, if you tweaked the configuration as in my comment on that issue) to 18.4kB :
.
Looking at a comparison of the actual source change between v2.0.4 and v3.0.0, with e.g.
git diff v2.0.4..39a52dceb0 src/autocomplete.js
(all the other src/ changes are small), there doesn't appear to be anything in there that would explain such a large increase, especially as you say you're also dropping old-IE polyfills. There's only the changes you outline in the release notes (plus #676 which appears to be missing, hope that's helpful).I have applied the PRs (bar the additional classes which didn't cherry-pick cleanly in the time I had available) on top of my fork, it's still only 8kB and seems to work fine: https://github.com/mysociety/accessible-autocomplete/tree/v2.0.4-ms2
Diffing
src
shows the missing "additional classes" code, but that's all; diffingdist
shows the expected changes - but the v3.0.0 file has an awful lot of extra code up front. What it appears to be doing is including a large number of core-js polyfills, e.g. Array.prototype.push (and all its internal dependents) allegedly in order to deal with the esoteric edge case of https://stackoverflow.com/a/77556853/669631. I'm unclear as to the purpose of including these polyfills in a library like this (does it really need to include a polyfill for Array.prototype.join?), that almost certainly doesn't need any of them, and more than doubles the size of the gzipped library (never mind the ungzipped). I wonder if there's a way to tweak the configuration so as not to include them?The text was updated successfully, but these errors were encountered: