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

DP-8334: Use NPM instead of Bower to pull in front end dependencies #502

Merged
merged 3 commits into from
Mar 18, 2019

Conversation

rbayliss
Copy link
Member

@rbayliss rbayliss commented Mar 8, 2019

Description

This PR removes switches from using Bower to NPM for dependencies in Mayflower Patternlab. The motivation for doing this is that Bower is deprecated (see https://bower.io/), and Github does not do scanning of Bower dependencies for security vulnerabilities. I tried to keep versions as close as I could, but in some cases I wasn't able to maintain the version or we needed to switch to a completely different package. Things that are different:

  • I have no clue where we got our basic polyfills from, but we're now using a combination of core-js (for ES level polyfills), and mdn-polyfills (for browser level polyfills). These should be comparable, because of the nature of polyfills.
  • Specialized polyfills (fetch, UrlSearchParams, and picturefill) - have also been switched to what are probably new sources, but the new sources we're using are the most canonical ones I could find.
  • TwigJS - went from a claimed version of 0.10.5 to 1.13.2, although this isn't actually as extreme as it seems, since twigjs-bower used Twig 1.10.4.
  • Fitvids - it was much easier to switch to the vanilla JS version of fitvids than to figure out how to import the jQuery fitvids plugin. It seems to work the same though, so I think we should be fine here.

Additionally, at the end of this work, I ran npm audit to fix known vulnerabilities. I believe the only things that were updated were build related (lots of minimatch updates).

The overall effect of this PR is that it increases our vendor JS size from 332K to 396K, which I consider an acceptable tradeoff for now. We could reduce this further in the future by fully removing Handlebars (~100k, which we're barely using), and by splitting off the polyfills into a file that's only loaded for older browsers (50k).

Related Issue / Ticket

Steps to Test

  1. Using IE 11, test the following pages in the Mayflower preview, interacting with the UI and just generally trying to break things:
    • Event Listing
    • Location Listing
    • Rules of Court
    • Press Listing

Screenshots

Use something like licecap to capture gifs to demonstrate behaviors.

Additional Notes:

Anything else to add?

Impacted Areas in Application

@todo

Today I learned...

@rbayliss rbayliss changed the title Replace bower with NPM in Patternlab DP-8334: Use NPM instead of Bower to pull in front end dependencies Mar 8, 2019
Copy link
Contributor

@ygannett ygannett left a comment

Choose a reason for hiding this comment

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

LGTM.
Tested in:
Windows 7

  • IE 11
  • FF 65

Mac OS 10.14.3

  • Chrome72

Comparing to the current version (V9.2.1), the found display issues are not related to this change.
They are either global or IE specific css issues, which should be handled separately.

No behavior issues are found in any pages in all browsers listed above even after interacting with the UI.

@ygannett ygannett merged commit f82ddaa into develop Mar 18, 2019
@clairesunstudio clairesunstudio mentioned this pull request Mar 20, 2019
@avrilpearl avrilpearl deleted the feature/DP-8334-remove-bower branch April 11, 2019 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants