Add "check-javascript-syntax" CircleCI build task

[aduth]

Why: To help avoid issues where JavaScript code which is not supported in legacy browsers could unknowingly be included in the built assets.

See:

• Include USWDS dependencies in Babel transformations #587
• Add CircleCI lint step to verify ES5 syntax for Internet Explorer 18F/identity-idp#4296

Changes temporarily include a revert of #587 to verify that the build fails as expected. This should be re-reverted prior to merging.

[aduth]

Fails in CircleCI as expected:

#!/bin/bash -eo pipefail
npm run es5-safe

> identity-site@1.0.0 es5-safe /home/circleci/project
> find assets/js/build -name '*.js' -exec cat {} \; | acorn --ecma5 --silent

The keyword 'const' is reserved (2:95470)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! identity-site@1.0.0 es5-safe: `find assets/js/build -name '*.js' -exec cat {} \; | acorn --ecma5 --silent`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the identity-site@1.0.0 es5-safe script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/circleci/.npm/_logs/2021-03-17T22_39_16_534Z-debug.log


Exited with code exit status 1
CircleCI received exit code 1

[aduth]

Revert the revert in 286bbe5.

[zachmargolis]

LGTM

[zachmargolis]

We have a lot of parallel build steps now.

Our JS build is fairly fast, so maybe not worth it here, but this is duplicated from our build-site command. Maybe would it be worth having a multi-phase job where we build the site once in build-site and then save the workspace and load that workspace in all the jobs that run the various checks?

So it would look like:

[build site, cache artifact] ->
  - run JS lint
  - run JS specs
  - run ruby specs on built site, etc etc

[zachmargolis]

Some example of workspace saving across steps in the old handbook: https://github.com/18F/identity-handbook-private/blob/master/.circleci/config.yml#L47-L59

[aduth]

Yeah, that sounds really great. I've tried incorporating this for "base" tests, as well as the external links checking which has a similar need. The timings aren't quite as promising as I'd have hoped, and in-fact the builds take slightly longer than before I'd incorporated it.

Still, it seems like a good idea in theory, and might help some future scalability, so I'd be fine to keep the changes 🤷

[zachmargolis]

Awesome! thanks for digging in to all that! I'm OK keeping these changes if you are

[zachmargolis]

do we want to persist the node_modules etc too? or do we assume that caching will take care of that for us

[aduth]

do we want to persist the node_modules etc too? or do we assume that caching will take care of that for us

I did that at first, and I found that it took a very long time for the persist_to_workspace step itself to complete, probably because node_modules is a black hole. And yeah, was hopeful that the caching in install-dependencies could keep it fast enough.

See: fe78bdf