Skip to content
This repository has been archived by the owner on Mar 31, 2023. It is now read-only.

115: ES6 support, ESLint, Babel 7 Upgrade and Babel Polyfill #322

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

evanmwillhite
Copy link
Contributor

@evanmwillhite evanmwillhite commented Feb 4, 2019

115: Add ESlint for JS

This PR changed from addressing merely ESLint to moving to ES6 syntax support for all default code.

Companion PR in Emulsify Gulp fourkitchens/emulsify-gulp#108. Must be merged/deployed at the same time.

Changes/additions:

  • Updates to Babel 7 (Emulsify Gulp PR)
  • Adds ESLint to deps (Emulsify Gulp PR)
  • Adds ESLint config files (ignore and rc)
  • Adds Babel config file
  • Adds Babel polyfill (since we/Drupal support IE11 out of the box)
  • Updates script files to ES6 Syntax!

To Test:

  • Checkout this branch in your local Emulsify repo
  • If you can use npm link to test the Emulsify Gulp PR (I couldn't get it to work), make sure you've done that and checked out the eslint-dep branch there. If it doesn't work for you either, replace ^4.0.0 with fourkitchens/emulsify-gulp#eslint-dep in this line of package.json (please don't commit the change) and run yarn.
  • Run yarn start and ensure there are no errors and that the JS components (accordion, main menu (mobile) and tabs) work correctly in all browsers including IE11
  • Ensure no JS console errors
  • Verify code looks up to snuff

Copy link

@patrickocoffeyo patrickocoffeyo left a comment

Choose a reason for hiding this comment

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

I left a couple thoughts in here, I'm not the best front end javascript person so it's worth getting other eye balls on this :)

One additional note, I noticed you have both a yarn.lock and a package-lock.json file in this repo. If these get out of sync at any point, it could create some weird side effects and conflicts. You might want to consider standardizing on either yarn or npm.

@waako
Copy link

waako commented Mar 5, 2019

This would be amazing, I'm really missing ESLint nagging me about how to improve/fix my javascript.

@amazingrando
Copy link
Member

@evanmwillhite Passes code review. After someone functional tests, we're good.

@@ -0,0 +1,3 @@
> 0.5%
not dead
ie 11

Choose a reason for hiding this comment

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

If you're using browserlist, I think the emulsify-gulp gulp-css.js and gulp-config.js files needs to be updated, so you won't pass browser arguments through the cssConfig.autoPrefixerBrowsers variable anymore?

Maybe it could be augmented so the variable will accept the autoprefixer options? like {grid: autoplace}?

https://github.com/fourkitchens/emulsify-gulp/pull/108/files

@amazingrando amazingrando removed their assignment Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants