Skip to content

Add build-time ES5 validation to fallback build.#36596

Merged
sgomes merged 1 commit intomasterfrom
update/validate-es5-in-fallback-build
Oct 8, 2019
Merged

Add build-time ES5 validation to fallback build.#36596
sgomes merged 1 commit intomasterfrom
update/validate-es5-in-fallback-build

Conversation

@sgomes
Copy link
Copy Markdown
Contributor

@sgomes sgomes commented Oct 8, 2019

ES6+ code sometimes finds its way into the fallback build, which should only include ES5, when one of our dependencies decides to ship their package with newer syntax.

This PR adds a new build-time check that ensures no non-ES5 syntax is present in the generated fallback build, breaking the build if that's the case. This will help us catch these issues as soon as they happen, rather than when an IE11 user reports them.

This PR also fixes all existing instances of this problem.

Thanks to @simison for suggesting the command line check! 👍

Changes proposed in this Pull Request

  • Add validate-fallback-es5 npm script to fallback build process
  • Add all existing instances of non-ES5 libraries to nodeModulesToTranspile

Testing instructions

Ensure the build doesn't fail. For extra points, ensure that devdocs works correctly in IE11 now (or at least it doesn't complain about invalid syntax).

@sgomes
Add build-time ES5 validation to fallback build.
cbae314 
ES6+ code sometimes finds its way into the fallback build, which should
only include ES5, when one of our dependencies decides to ship their
code with newer syntax.

This PR adds a new build-time check that ensures no non-ES5 syntax is
present in the generated fallback build, breaking the build if that's
the case. This will help us catch these issues as soon as they happen,
rather than when an IE11 user reports them.

This PR also fixes all existing instances of this problem.
@sgomes sgomes added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Build labels Oct 8, 2019
@sgomes sgomes requested review from a team, simison and sirreal October 8, 2019 13:52
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Oct 8, 2019

Test live: https://calypso.live/?branch=update/validate-es5-in-fallback-build

@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Oct 8, 2019

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Async-loaded Components (~66 bytes removed 📉 [gzipped])

Details 
name                          parsed_size           gzip_size
async-load-design-playground       +107 B  (+0.0%)      -22 B  (-0.0%)
async-load-design-blocks           +107 B  (+0.0%)      -22 B  (-0.0%)
async-load-design                  +107 B  (+0.0%)      -22 B  (-0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

blowery
blowery approved these changes Oct 8, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@blowery blowery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful! This will save so much time and frustration.

@sgomes
Copy link
Copy Markdown
Contributor Author

sgomes commented Oct 8, 2019

Thank you for the review and kind words, @blowery! 😄

@sgomes sgomes merged commit 519246c into master Oct 8, 2019
@sgomes sgomes deleted the update/validate-es5-in-fallback-build branch October 8, 2019 14:23
@simison
Copy link
Copy Markdown
Member

simison commented Oct 8, 2019

I wonder if this could be useful in Gutenberg as well?

@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 8, 2019
@sgomes
Copy link
Copy Markdown
Contributor Author

sgomes commented Oct 8, 2019 •
edited
Loading

I wonder if this could be useful in Gutenberg as well?

@simison I would expect this to be useful in any project that both:

  • Doesn't transpile its npm dependencies by default
  • Expects its bundle(s) to run in ES5-only browsers

@sirreal
Copy link
Copy Markdown
Member

sirreal commented Oct 8, 2019

@Automattic/jetpack-crew You might like to grab this linter for compiled bundles in Jetpack. It should help prevent ie11 breakage.

@jeherve jeherve mentioned this pull request Oct 9, 2019
Closed
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Oct 9, 2019

Created an issue to track this here: Automattic/jetpack#13693

This was referenced Sep 9, 2020
Merged
Merged
ockham added a commit to Automattic/jetpack that referenced this pull request Oct 21, 2020
@ockham
Build: Ensure that production builds are valid ES5 (#17127)
5d6f520 
Add ES5 validation to production builds.

Inspired by Automattic/wp-calypso#36596.

Can't currently add it to the `extension` build, since there are non-ES5 constructs in them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blowery blowery blowery approved these changes

@simison simison Awaiting requested review from simison

@sirreal sirreal Awaiting requested review from sirreal

Assignees

No one assigned

Labels

Build Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sgomes @matticbot @simison @sirreal @jeherve @blowery