Page MenuHomePhabricator

Investigate common.js bundle content/size
Open, Needs TriagePublic

Description

The common.js build of bridge does not seem to receive app-build-level optimization.
It is not minified and contains comments - which has certain benefits (think: debugging) and should be mitigated by ResourceLoader.
However, it contains information that is not essential to the app (e.g. the entirety of HTTP status codes, even the ones not used in the app). How can we avoid this?

Findings should also be communicated to the termbox team for T228528: Use externalized vue.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2019, 11:52 AM
Pablo-WMDE updated the task description. (Show Details)Aug 30 2019, 5:30 PM

There’s a fairly simple way to see what webpack is including in the built files (I’m checking the init module here because I’m doing this as part of investigating T233305, but the app module should work similarly):

node_modules/.bin/vue-cli-service build --mode development
grep -F '!*** ' dist/data-bridge.init.js

I’ve pasted the full list at P9267 – mostly, it’s a whole bunch of core-js polyfills, including for things like Function.prototype.bind which we are really, really guaranteed to have already (by MediaWiki) and things like RegExp.prototype[@@match] that we never use directly. That list also doesn’t change if I trim the .browserslistrc, so I’m not sure where @babel/preset-env gets its browser list from that it thinks we need to be compatible with.

I looked into the bind thing a bit further, and as far as I can tell the chain of dependencies is:

  • ./node_modules/core-js/library/modules/_bind.js
  • is imported by ./node_modules/core-js/library/modules/es6.reflect.construct.js
  • is imported by ./node_modules/core-js/library/fn/reflect/construct.js
  • is imported by ./node_modules/@babel/runtime-corejs2/core-js/reflect/construct.js
  • is imported by ./node_modules/@babel/runtime-corejs2/helpers/esm/construct.js
  • is imported by ./node_modules/@babel/runtime-corejs2/helpers/esm/wrapNativeSuper.js
  • is imported by:
    • ./src/data-access/error/EntityNotFound.ts
    • ./src/data-access/error/EntityWithoutLabelInLanguageException.ts
    • ./src/data-access/error/JQueryTechnicalError.ts
    • ./src/data-access/error/TechnicalProblem.ts

Three of those last four classes are completely empty, by the way. But Babel still thinks their (implicit) constructors need to “wrap native super”, whatever that is, which requires four levels of Reflect.construct shims, which ultimately require Function.prototype.bind. And at no point in this chain are there any checks if the browsers list already includes the needed code – these are all regular, unconditional require() imports.

As far as I can tell, @babel/preset-env is good for checking which parts of core-js your own code actually needs, and only including those parts – but then the various parts of core-js (and @babel/runtime-corejs2, apparently) still merrily depend upon one another, so that you still end up pulling in a lot of code that you don’t actually need.

@Lucas_Werkmeister_WMDE With regards to browserslist behaving funny: try, to rule something out, to run the build in an env where the directories from data-bridge up are not present.

Thanks, that’s… good to know… but I copied data-bridge into /tmp and it looks like it’s still generating the same modules. I think it’s because of the cross-module imports that I mentioned in the second half of the comment, not because of the browsers list directly.

I spent what felt like (and maybe even was) hours on this only to be hit at the head with the realization that npm/node for some unfathomable reason caches the contents of .browserlistrc and babel.config.js. This cache can be deleted with
$ rm -rf node_modules/.cache

Then changes to .browserlistrc are correctly recognized for me.

I made this exact mistake a few months ago when the hike started and forgot about it again 🤦. Now there is a post-it at my monitor. Maybe it helps.