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

build(js,css): add browserslist spec and update JS and CSS toolchains #4083

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Sep 18, 2019

overview

Following up on our cross-browser support discussion(s), this PR implements JS polyfilling and improves out CSS polyfilling support

DO NOT MERGE until after PD's next release PD release, merge unblocked

changelog

  • Adds a .browserslistrc file to specify our JS and CSS polyfill support
  • Adds @babel/preset-env polyfilling via core-js to our three main web-apps (app, PD, LL)
  • Upgrades JS / CSS build toolchains across the board
  • Switches off the discontinues cssnext to postcss-preset-env
  • Makes a few code changes to support the toolchain changes
    • CSS color function renamed to color-mod
    • stable module added for guaranteed stable array sorting in discovery-client
      • Array.prototype.sort is not guaranteed stable and the addition of core-js was somehow exposing this and causing tests to go flakey

review requests

This is marked as do not merge until the planned bugfix release of PD goes out at the end of this week. Everything looks good in my cursory checking, but given the kinds of changes this PR makes, App, PD, and LL should really go through each of their full QA suites before the next release with this change.

The best testing you can do is just stress testing. Be on the lookout for:

  • Weird errors in the console that weren't there before
  • Colors or other styles not getting applied properly
  • Unexpected white-screens
  • Unexpected test failures, flakey or otherwise

@mcous mcous added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review protocol designer Affects the `protocol-designer` project labware DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available labels Sep 18, 2019
@codecov
Copy link

codecov bot commented Sep 18, 2019

Codecov Report

Merging #4083 into edge will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #4083      +/-   ##
==========================================
+ Coverage   57.24%   57.24%   +<.01%     
==========================================
  Files         850      850              
  Lines       24004    24002       -2     
==========================================
  Hits        13741    13741              
+ Misses      10263    10261       -2
Impacted Files Coverage Δ
babel.config.js 0% <ø> (ø) ⬆️
discovery-client/src/service-list.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5595f8d...7cd6ea5. Read the comment docs.

Copy link
Contributor

@IanLondon IanLondon left a comment

Choose a reason for hiding this comment

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

code looks good, PD and LL+LC both seem like they're working fine and everything looks the same to me, no new console messages. Couldn't find any missed color(s in CSS of PD/App/LL.

@mcous mcous removed the DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available label Sep 19, 2019
Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

  • RA layout/colors appear unaffected
  • LL and LC layout/colors appear unaffected
  • CompLib layout/colors appear unaffected

@mcous mcous merged commit 33a4012 into edge Sep 20, 2019
@mcous mcous deleted the chore_browserslist branch October 9, 2019 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project feature Ticket is a feature request / PR introduces a feature labware protocol designer Affects the `protocol-designer` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants