ES5 shim cleanup

[anselmbradford]

The ES5 shim/sham script was checking ES5 support twice in modernizer. Once in the base template and once in the es5-shim module. IE8 shouldn't need the shim because we turn off JS in IE8 (though in theory GTM could be using ES5 features, but if that happens the GTM script should be refactored to not use ES5 features, as they shouldn't require the amount of complexity that needs ES5 features). IE9 supports ES5 features except for strict-mode, which es5-shim does not polyfill anyway, so we don't need to deliver the shim there. IE10+ support es5 features, so shim doesn't need delivery there.

Update: Completely removes es5-shim.

Changes

• Renames scriptsEs5Shim build function scriptsES5Shim to match ECMAScript case.
• Renames common.ie.js to common.ie9.js to better convey that that is the only IE this module is delivered to.
• Moves class-list polyfill into common.ie9.js, since that's the only place it was used.
• Renames es5-shim.js to es5-shim-sham.js to better convey what it contains.
• Completely turn off delivery of es5-shim to conditional comment supporting IE versions.
• Removes redundant es5 feature check inside es5 shim module.
• Removes fnBind polyfill, since it is only needed for IE8 and BB7. IE8 shouldn't need the polyfill because the JS where it's used is turned off. BB7 should fail the modernizr es5 check and use the bind polyfill inside the es5 shim.
• Turns off JS for browsers that don't support es5 features, according to modernizr.

Removes

• Completely removes es5 shim.

Testing

1. gulp build should pass.
2. No new errors in IE8 (has youtube and GTM errors) and IE9+

Notes

• Includes Remove ESLint ignore file #3675 and No mobile view on IE9 #3678
• You can check browser support for es5 features with finer control with the following logs:

    console.log( 'es5array:' + window.Modernizr.es5array );
    console.log( 'es5date:' + window.Modernizr.es5date );
    console.log( 'es5function:' + window.Modernizr.es5function );
    console.log( 'es5object:' + window.Modernizr.es5object );
    console.log( 'strictmode:' + window.Modernizr.strictmode );
    console.log( 'es5string:' + window.Modernizr.es5string );
    console.log( 'es5syntax:' + window.Modernizr.es5syntax );
    console.log( 'es5undefined:' + window.Modernizr.es5undefined );

Todos

• At some point if we want to drop BB7 es5 feature support we could remove es5-shim completely. @ascott1 @cfarm ? Dropping it!

[sebworks]

We don''t support Blackberry and we should remove the shim. I think were also using it for Android 4.x as well. You can't tell me this works on Blackberry because no here can test it.

[anselmbradford]

I've been lazy about the iphone update, so still have BB7. We could remove the shim, deploy, and I could report back what happens in prod and then we can decide if we care. That work?

[sebworks]

I've been lazy about the iphone update, so still have BB7. We could remove the shim, deploy, and I could report back what happens in prod and then we can decide if we care. That work?

No, we should drop support for it because we don't support it. I am seeing near 50 BB7 users since July 1, 2017. The only concern I have is the Android 4.x users.

[anselmbradford]

You give a convincing argument! Okay, I've re-worked it to use modernizr to check for es5 features (aside from strict mode—to handle IE9) and if they are absent, disable JS as we do in IE8. I checked in SauceLabs Android 4.4 (lowest I think they have) and ES5 features were supported. Also, contains #3678

[jimmynotjim]

Should we change the footer script to do the same instead of relying on the IE 8 comment hack? https://github.com/cfpb/cfgov-refresh/pull/3677/files#diff-89b43ee86bbd4952dc44cf751cfd775cR247

[anselmbradford]

Okay check now

[anselmbradford]

@sebworks Per Jimmy's feedback to change the check in the footer to match, I wrapped the footer scripts in an if statement looking for the no-js class. This required injecting the script elements instead of <script>. For the component JS I realized we don't need to inline and use django compression on it any more with http/2. Is that correct?

[sebworks]

I see now, the problem with this approach is that it's a lot slower for folks who are using JS because you have to wait for it to create and attach the scripts. 

[jimmynotjim]

the problem with this approach is that it's a lot slower for folks who are using JS because you have to wait for it to create and attach the scripts.

How do you feel about moving it to the header and making the load async to speed it up for JS users without blocking page load but also avoid loading any scripts for no JS users?

[jimmynotjim]

Is this indentation change intentional? HTML files should still be four space, even if writing JS

[anselmbradford]

It was intentional. Once you're inside <script> it's JS land, no?

[jimmynotjim]

I don't think so. If we linted it, the linter would only care that the file was html, not what the code written inside of it was.

[anselmbradford]

Okay I reverted it, but I think a smart HTML linter wouldn't be touching what's inside a <script> tag because it's not HTML. If we used something like https://www.npmjs.com/package/eslint-plugin-html#htmlindent, we could control it however we want, but it would still be JS and JS code style I'd think.

[anselmbradford]

This is the unminified script:

function loadScript( url, callback ) {
  var script = document.createElement( 'script' );
  script.type = 'text/javascript';

  script.onload = function() {
    if ( callback ) {
      return callback();
    }
    return null;
  };

  script.src = url;
  document.getElementsByTagName( 'head' )[0].appendChild( script );
}

[anselmbradford]

Actually removed the callback onload bit, as none of the scripts use a callback. 25cf9ad

[jimmynotjim]

Builds fine and looks good in my IE8 VM.

[anselmbradford]

Thanks for fixing Travis @sebworks !