Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Upgrade to babel 7 #579

Merged
merged 14 commits into from
Nov 6, 2018
Merged

Upgrade to babel 7 #579

merged 14 commits into from
Nov 6, 2018

Conversation

cometkim
Copy link
Contributor

Summary

Upgrade babel to 7 beta. (many other projects are already moved e.g. React Native)

See https://babeljs.io/docs/en/next/v7-migration

  • babel core is peer dependency
  • stage-x, es201x has been deprecated
  • drop unused polyfills with useBuiltIns: "usage"
  • much faster

Looks just fine for this package, but need to prepare for webapp and mobile either.

Ticket Link

None

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit tests passed
  • Ran make flow to ensure type checking passed
  • Added or updated unit tests (required for all new features)
  • All new/modified APIs include changes to the drivers

Test Information

This PR was tested on: Ubuntu Xenial

@enahum
Copy link
Contributor

enahum commented Jul 23, 2018

React native made the move for 0.56 but we are still using 55 and no plans for upgradig yet

@jasonblais
Copy link
Contributor

Hi @cometkim

First, really appreciate your help with the refactoring and redux migration here: mattermost/mattermost-webapp#1666

You have a great approach and are taking a comprehensive look at all the possible cases through your matrix 👍

Second, we have recently discussed moving the webapp to use Babel 7. Part of this will be to determine if the ability to use native browser implementations for features and only polyfill when necessary will have significant performance improvements.

Given you've started the Babel 7 upgrade for redux, wondering if you were interested in participating in the effort to move the webapp to Babel 7? If so, I'd be happy to connect you with our team when you have time for it :)

@enahum
Copy link
Contributor

enahum commented Sep 21, 2018

BTW we just upgraded to RN 0.57.0 that uses babel 7

@cometkim
Copy link
Contributor Author

Sounds great! Of course I interested to. actually already has tried to upgrade webapp project to use Babel 7 once, It has failed to build.

Anyway, I will resume this PR first (maybe this weekend..?). I think it wouldn't affect mobile and webapp project.

Thank you for notice @enahum, @jasonblais. please let me know if you started for webapp or mobile project :)

@jasonblais
Copy link
Contributor

Thanks @cometkim! Sounds great.

Given the mobile projects is now using react native v57, it contains the babel 7 upgrade. But once we've merged the Redux PR, we can work together on adding babel 7 for the webapp project.

@cometkim cometkim force-pushed the babel-7 branch 2 times, most recently from a733079 to 4ed2391 Compare September 22, 2018 15:07
@cometkim
Copy link
Contributor Author

@jasonblais @enahum Ready to merge.

Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

Awesome! That was quick

@jasonblais
Copy link
Contributor

@cometkim We're working on moving webapp to babel 7 now as well

@cometkim
Copy link
Contributor Author

cometkim commented Oct 7, 2018

How is it going? It seems you've done at mattermost/mattermost-webapp@a825d5c.

@cometkim
Copy link
Contributor Author

cometkim commented Oct 7, 2018

  • Mocha / Chai / Nock
  • ESLint
  • Flow
  • React
  • reselect
  • Webpack
  • ws

are also need to be updated.

@jasonblais
Copy link
Contributor

@cometkim Do those need to be updated on the webapp or redux repository?

@cometkim
Copy link
Contributor Author

cometkim commented Oct 7, 2018

@jasonblais only on the redux repo and each can be handled in other PRs.

package.json Outdated
"babel-core": "6.26.3",
"babel-eslint": "8.2.3",
"babel-loader": "7.1.4",
"@babel/cli": "^7.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we set all these to be exact please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@enahum yep.

@cometkim
Copy link
Contributor Author

@enahum Any updates?

@enahum enahum requested a review from crspeller October 31, 2018 16:39
@enahum enahum added the 2: Dev Review Requires review by a core commiter label Oct 31, 2018
// See LICENSE.txt for license information.

module.exports = {
presets: [
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity why it should match the webapp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crspeller I updated it to match with webapp's.

but I think that mattemrost-redux not only for mattermost-webapp and mattermost-mobile, but also it is a reference client implementation of the API. So future, We need to support a bit more compatibility and other specs like es module.

Copy link
Member

Choose a reason for hiding this comment

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

I was just asking about browser support. Which should probably be unified across our repositories. I agree that our support for this repository should be more broad.

@cometkim cometkim force-pushed the babel-7 branch 2 times, most recently from 3a7ba48 to a31cb31 Compare November 6, 2018 13:21
@crspeller crspeller merged commit 9c50b6b into mattermost:master Nov 6, 2018
DSchalla pushed a commit to DSchalla/mattermost-redux that referenced this pull request Apr 1, 2019
* Upgrade to babel 7

* Disable module transform in webpack build

* Bump up versions of Babel deps

* Update Babel 7 configuration file

- Change config file to babel.config.js
- Add class-properties proposal
- Enable loose mode

* Disable module transform by default, is it better?

* Revert "Disable module transform by default, is it better?"

This reverts commit 31eb0f5.

* Fix mocha setup

* Disable loose mode, it breaks test

* Fix webpack config

* Update babel dependencies

* Fix devDeps with exact version notation

* Fix target settings of babel/env preset to match with webapp

* Set debug true if not production build

* Ignore eslint on babel config
@cometkim cometkim deleted the babel-7 branch March 21, 2020 00:06
chetanyakan pushed a commit to brightscout-alpha/mattermost-redux that referenced this pull request Feb 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2: Dev Review Requires review by a core commiter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants