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

upgrade to babel 7 and other dependencies #210

Closed
wants to merge 2 commits into from

Conversation

iAziz786
Copy link

This pull request upgrades babel to v7. In order to make everything compatible, I had to upgrade other dependencies as well.

Coverage drops to around 69.23%. @jasonslyvia Can you help to improve it?

Refers to #207

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.7%) to 69.231% when pulling 158dd74 on iAziz786:upgrade-dependencies into 80bcae4 on jasonslyvia:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.7%) to 69.231% when pulling 158dd74 on iAziz786:upgrade-dependencies into 80bcae4 on jasonslyvia:master.

@coveralls
Copy link

coveralls commented Oct 26, 2018

Coverage Status

Coverage decreased (-6.7%) to 69.231% when pulling f2081c1 on iAziz786:upgrade-dependencies into 80bcae4 on jasonslyvia:master.

],
"plugins": [
"@babel/plugin-syntax-dynamic-import",
"@babel/plugin-proposal-class-properties",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not quite familiar with the Babel stack now, can you explain why these two plugins are required?

Copy link
Author

Choose a reason for hiding this comment

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

Both are official Babel plugins. Since Babel 7, Babel is trying to move to a scope system that's why I have to prefix @babel to the plugin names.

@babel/plugin-syntax-dynamic-import: is used for situations for using dynamic imports import() syntax; I guess I forgot to remove this as we're not doing any.

@babel/plugin-proposal-class-properties: is for new class fields where we can define any property on the class as a regular assignment. This one is required, because, instead of using ::this.handleSomeChange to bind this, I'm using the class fields and arrow functions to do so. The syntax is cleaner and more readable. This may look personal preference but class fields syntax is at stage-3 and more likely to land to JavaScript world whereas :: binding which is at stage-0.

We can write this:

class MyClass {
  ...
  handleSomeChange() {
    doSomething(this.state.value);
  }
  ...
  render() {
    return (
      <button onClick={::this.handleSomeChange}>Click</button>
    )
  }
}

To this:

class MyClass {
  ...
  handleSomeChange = () => {
    doSomething(this.state.value);
  }
  ...
  render() {
    return (
      <button onClick={this.handleSomeChange}>Click</button>
    )
  }
}

I guess that explains why I have added this plugin.
I'm removing @babel/plugin-syntax-dynamic-import because we're not using it anyway; Thanks for pointing out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for your excellent explanation!

@jasonslyvia
Copy link
Collaborator

jasonslyvia commented Oct 29, 2018

I'm okay with the overall change, @ameerthehacker any thoughts?

@iAziz786 iAziz786 closed this Nov 13, 2021
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.

None yet

3 participants