Skip to content
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

Building Tools: add build-time ES5 validation to build #13693

Closed
jeherve opened this issue Oct 9, 2019 · 7 comments · Fixed by #17127
Closed

Building Tools: add build-time ES5 validation to build #13693

jeherve opened this issue Oct 9, 2019 · 7 comments · Fixed by #17127
Assignees
Labels
Build [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Comments

@jeherve
Copy link
Member

jeherve commented Oct 9, 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.

Calypso added 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. It would be nice to add this to Jetpack as well. This will help us catch these issues as soon as they happen, rather than when an IE11 user reports them.

npx eslint --env=es5 --no-eslintrc --no-ignore ./_inc/build ./_inc/blocks

-- Automattic/wp-calypso#36596

@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it Build labels Oct 9, 2019
@simison
Copy link
Member

simison commented Oct 9, 2019

I wonder if this would make it possible to remove forced wp-polyfill dependency from Gutenberg extensions... 🤔

@simison
Copy link
Member

simison commented Oct 9, 2019

...or even better, make the core's dependency extraction plugin auto-detect if script needs wp-polyfill 🤔 cc @gziolo @sirreal

@gziolo
Copy link
Member

gziolo commented Oct 9, 2019

...or even better, make the core's dependency extraction plugin auto-detect if script needs wp-polyfill 🤔 cc @gziolo @sirreal

That would be neat, I have no idea how feasible it is.

As far as I know, IE11 isn't the biggest issue here as it has a known set of limitations even in terms of full ES5 support. There are also other browsers which are popular and have to be supported.

If you know how to solve it, feel free to open PR.

@simison
Copy link
Member

simison commented Oct 9, 2019

As far as I know, IE11 isn't the biggest issue here as it has a known set of limitations even in terms of full ES5 support. There are also other browsers which are popular and have to be supported.

https://github.com/Volox/eslint-plugin-ie11 looks promising:

ESLint plugin for detecting unsupported ES6 features in IE11

https://github.com/amilajack/eslint-plugin-compat seems even better and relies on browserslist:

Lint the browser compatibility of your code

@stale
Copy link

stale bot commented Apr 7, 2020

This issue has been marked as stale. This happened because:

  • It has been inactive in the past 6 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@ockham
Copy link
Contributor

ockham commented Sep 10, 2020

Let's add at least ES5 validation for starters -- it's better to have some validation rather than none at all, to prevent things like class from going untranspiled. FWIW, Automattic/wp-calypso#36596 seems to have served us well in Calypso. We can fine-tune later if we discover that there's still things that IE11 doesn't "understand".

I'll spin up a PR.

@stale stale bot removed the [Status] Stale label Sep 10, 2020
@simison
Copy link
Member

simison commented Sep 10, 2020

Just heads-up that viable alternative to eslint could be using Acorn directly:

acorn.parse(bundleContent, { ecmaVersion: 5 });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants