LG-3764, LG-3716: Use identity-style-guide@3 to deduplicate zxcvbn dependency

[aduth]

Why: To use latest USWDS version consistently across all login products and services.

Why: The zxcvbn dependency is large and should not be included in JavaScript bundles where it is not used. Instead, bundles should use the minimum implementations necessary. This is made possible in the latest version 3.0 of identity-style-guide.

Related: 18F/identity-design-system#165 (LG-3757)

Impact:

Treemap:

Before	After

Highlighted segments identify where zxcvbn is loaded. Previously identity-style-guide was prebundled, so it was not clearly labelled separate, but you can tell by the fact that identity-style-guide inflated bundle size significantly.

Bundle Size:

Before:

   js/application-3ab408e425de3e0bfa39.js    1.02 MiB    2, 3  [emitted] [immutable]  [big]  application
js/application-3ab408e425de3e0bfa39.js.br     406 KiB          [emitted]              [big]  
js/application-3ab408e425de3e0bfa39.js.gz     457 KiB          [emitted]              [big]  

After:

   js/application-95e84262cf8ef6113d8c.js     221 KiB    2, 3  [emitted] [immutable]         application
js/application-95e84262cf8ef6113d8c.js.br    53.8 KiB          [emitted]                     
js/application-95e84262cf8ef6113d8c.js.gz    63.3 KiB          [emitted]                     

[aduth]

We probably should have been using this all along:

https://github.com/uswds/uswds#sass-compilation-requirements

It's more obviously necessary now, as without it, the caret in the USA banner does not display correctly:

Related: uswds/uswds#3860

[aduth]

This was previously misnamed, fixed between USWDS 2.8.0 and 2.8.1:

• File input preview class names are not valid BEM syntax uswds/uswds#3573
• uswds/uswds@de86e3b

[zachmargolis]

LGTM! Love the asset size decrease, nice work!

[zachmargolis]

awesome that we're able to clean all these TODOs up!

[mitchellhenke]

wow this is a great fix to decrease bundle size 👏🏼

[aduth]

Noticed a couple minor regressions in some further testing in mobile contexts:

1. We had quite a bit of custom styling for the banner, which wasn't compatible with the updated revisions. This was reconciled and simplified in 880381a, bringing closer in line with USWDS / Login Design System, while still customizing (a) the background color to white and (b) centering the text in desktop environments.

	Before (current)	After (proposed)
Collapsed
Expanded

2. The external link icon disappeared from the GSA link in the footer. This appears to stem from an upstream revision in USWDS which isn't inherently compatible with placing the icon in a flex container whose contents are vertically aligned. Eventually I landed on 2dee151 as a resolution.

[aduth]

Another issue seemingly stemming from the external link icon revision: In Safari, the footer links wrap even before they occupy the available width of the flex container:

Seems like it may possibly be related to behavior (bug?) specific to Safari where pseudo-element content is treated "as if they were real elements".

Looking at potential solutions:

• Prevent wrapping with white-space: nowrap on flex container
• Setting display: inline-block to each link
• Setting display: flex on links container
  • Most leaning to this at the moment, though it's made challenging by our use of !important with sm-hide and sm-show classes

[aduth]

Setting display: flex on links container

Implemented this in b119fd9.

[mitchellhenke]

* Setting `display: flex` on links container
  
  * Most leaning to this at the moment, though it's made challenging by our use of `!important` with `sm-hide` and `sm-show` classes

👍🏼

[aduth]

Discovered a small issue with how we build identity-style-guide that causes some ES6 code to be included, thus causing errors in IE11. This only affects usage of the package via NPM, not the prebundled main.js.

Specifically, we reference USWDS components by source, which can contain ES6 (example), and unlike the identity-style-guide's own source is not precompiled.

Follow-ups:

• Precompile USWDS components for identity-style-guide package build
  • Alternatively, we could explicitly allow USWDS files to be transpiled (prior art). This is a bit less work overall, but ultimately, it's good practice that an NPM package doesn't require downstream consumers to compile its source.
• Resolve the fact that CI didn't catch this, as catching these issues was the explicit purpose of Add CircleCI lint step to verify ES5 syntax for Internet Explorer #4296 (see Compile bundles for production in ES5 safety test #4494)

[aduth]

• Resolve the fact that CI didn't catch this, as catching these issues was the explicit purpose of Add CircleCI lint step to verify ES5 syntax for Internet Explorer #4296 (see Compile bundles for production in ES5 safety test #4494)

Rebased after merging #4296. Expecting build to fail now.

[aduth]

Expecting build to fail now.

Odd that it's not 🤔 It does locally:

NODE_ENV=production ./bin/webpack && yarn es5-safe
...
yarn run v1.22.5
$ is-es5-safe public/packs/js/*.js
public/packs/js/application-95e84262cf8ef6113d8c.js SyntaxError: The keyword 'const' is reserved (2:62633)

[aduth]

Expecting build to fail now.

Odd that it's not 🤔

We configure RAILS_ENV=test for CircleCI:

identity-idp/.circleci/config.yml

Lines 16 to 17 in e0bd0b9

	environment:
	RAILS_ENV: test

...where ./bin/webpack will output bundles to public/packs-test, not public/packs.

Trying ffd7cd7.

Edit: Success! And by "success", I mean the build is failing as expected 😄

[aduth]

Precompile USWDS components for identity-style-guide package build

• Alternatively, we could explicitly allow USWDS files to be transpiled (prior art). This is a bit less work overall, but ultimately, it's good practice that an NPM package doesn't require downstream consumers to compile its source.

After much toil yesterday afternoon, I eventually settled with the latter option here.

Ideally we don't distribute code which assumes downstream compilation, but since it originates from USWDS and its dependencies, any "fix" ultimately involves bundling a modified version of those dependencies, which can cause more problems than it helps.

410a261 seems to work in my testing. It's still more cumbersome than I'd hope, since there are a handful of these transitive dependencies, and because Babel defaults (sourceType) typically assume one is compiling their own code, consistently.

Open to other thoughts. I'd contemplated throwing my hands up and just bundling the precompiled version, but considering this is at the core of what LG-3716 aims to improve, it's not really an option.

There's a small semi-related note hidden in USWDS documentation:

If you require('uswds') in your bundled JS, you will get the minified ES5 browser bundle. If you're handling ES5 conversion already or using a tool that does it automatically, you can work around this in two ways:

https://github.com/uswds/uswds/tree/develop/examples#general

Follow-ups:

• Document this requirement in identity-style-guide
• Propose upstream improvements to USWDS to distribute transpiled source

[mitchellhenke]

👍🏼