Fix minified bundle

[cgewecke]

Description

NB: This PR targets the buildsize checker PR #3260, prerequisite for merging here

The bundle entry point is corrupted due to fix proposed in #3155 & implemented in #3206
There are two settings for the babel presets and in web3's case, picking either breaks the build in a different way.

	regeneratorRuntime error	"require" is not defined error
useBuiltIns: "usage"	false	true
useBuiltIns: "entry"	true	false

The fix is to use "entry" and add @babel/polyfill to gulp's browserify step. Increases the minified bundle's size about 150 kb (~10%). This wouldn't affect anyone whose packing things themselves though.

PR adds

• tests for the "require not defined" case (git repo and published bundles). The "runtime" error is already caught by the browser tests.

• an E2E validation which models the CDN use-case. It publishes a simple website which uses the bundle to a random url at surge.sh. This site uses Web3 to query the latest block from mainnet over Infura. An example from this PR's CI run is at:

  sudden-playground.surge.sh

(Sometimes surge.sh fails with an auth error because it's generated a name which is already taken or something. The tests will pass though - the surge check is just meant to be a convenience.)

The commits are sequenced with initially failing tests so their correctness can be checked:

• First commit 245b4ec has no build changes and reproduces the "require not defined" error here
• Second commit 72bf5f2 fixes "require not defined" but fails with the "runtime" error here
• Third commit 953c2e1 fixes both

Fixes #3256

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have selected the correct base branch.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• Any dependent changes have been merged and published in downstream modules.
• I ran npm run dtslint with success and extended the tests and types if necessary.
• I ran npm run test:unit with success and extended the tests if necessary.
• I ran npm run build-all and tested the resulting file/'s from dist folder in a browser.
• I have updated the CHANGELOG.md file in the root folder.
• I have tested my code on the live network.

[coveralls]

Coverage remained the same at 84.802% when pulling 38c5c54 on issue/3256 into f0f9ffc on ci/add-buildsize-checker.

[nivida]

The fix is to use "entry" and add @babel/polyfill to gulp's browserify step. Increases the minified bundle's size about 150 kb (~10%). This wouldn't affect anyone whose packing things themselves though.

entry is not only a fix it does also improve the way babel does add those imports and this should save some lines. An increase of 10% is in my view of point to much and should already show us that we do add to much stuff.
I've used the babel runtime and transform runtime plugin by side of the preset-env to be sure the required regenerator runtime is existing for each bundle and also to have just one absolute @babel/runtime package.

Could you elaborate on why exactly the usage or entry useBuiltIns options do make the difference as mentioned in the PR desc? As I know and only if babel does transpile our packages correctly should this not affect us in this way. The only difference is where exactly those imports are located (globally in the bundle or for each file).

Was just looking at the babel docs and honestly I don't know if useBuiltIns is the right option if the polyfill is being injected....

Yes, it isn't the correct thing. The goal would be to only use the useBuiltIns options and the corejs polyfills. The @babel/polyfill package you have added is already marked as deprecated by babel and I think we shouldn't rely on deprecated packages for this fix.

"Since @babel/polyfill was deprecated in 7.4.0, we recommend directly adding core-js and setting the version via the corejs option." - babel docs

[cgewecke]

@nivida

Could you elaborate on why exactly the usage or entry useBuiltIns options do make the difference as mentioned in the PR desc?

The clearest description I could find for this was in babel polyfill docs. But before I also tried many combinations of runtime plugins in the preset env block and could not get both tests passing. Maybe there's a way with usage and some kind of module option setting? Newer babel?

I've used the babel runtime and transform runtime plugin by side of the preset-env to be sure the required regenerator runtime is existing for each bundle and also to have just one absolute @babel/runtime package.

Please feel free to experiment with this! If you can get rid of the polyfill that would be incredible. The only thing I'd ask is that the tests remain as they are - they trigger the two errors we've seen.

An increase of 10% is in my view of point to much and should already show us that we do add to much stuff.

Agree, this is not a fix. It's a patch to stop something crashing. Over desktop the difference in loading the min over CDN with another 10% should be +25ms and it will not affect anyone bundling by themselves either.

Lastly - we agree the way forward is with rollup. If gulp will be ripped out within 1 month and the tests here are correct, then getting the babel setting perfect might be less important.

[nivida]

@cgewecke As already commented did I fix the require and regeneratorRuntime error with adding of the regenerator-runtime package in our gulp build pipeline and using of babelify. What I'm wondering is why the preset-env from babel is transforming the async call but doesn't add the required regenerator-runtime. Is this a bug from babel or is this thought to be the case? 🤔

Edit:
Using of the regenerator-runtime package directly instead of the @babel/polyfill package does increase the minified bundle size by 20Kb instead of 150Kb.

[cgewecke]

@nivida Ah great!! Thanks so much for figuring that out.

[cgewecke]

@nivida Couple additional notes:

• Rebased against 1.x
• e2e.cdn.sh now publishes to sudden-playground.surge.sh each time. It shouldn't fail anymore and you can change that to a site you control if your prefer via tokens in CI

[alcuadrado]

I re-reviewed this, and it looks really good now :)

[holgerd77]

Phew, what a detective work. Great!! 😄