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

Use native or lodash util module where full "lodash" is required #5042

Merged
merged 1 commit into from Jan 15, 2017
Merged

Use native or lodash util module where full "lodash" is required #5042

merged 1 commit into from Jan 15, 2017

Conversation

zertosh
Copy link
Member

@zertosh zertosh commented Dec 26, 2016

This PR ports all of the use sites of full lodash to either using the piecemeal utility modules or their native equivalents.

@codecov-io
Copy link

codecov-io commented Dec 26, 2016

Current coverage is 89.20% (diff: 86.36%)

Merging #5042 into master will decrease coverage by <.01%

@@             master      #5042   diff @@
==========================================
  Files           203        203          
  Lines          9821       9819     -2   
  Methods        1071       1071          
  Messages          0          0          
  Branches       2615       2615          
==========================================
- Hits           8761       8759     -2   
  Misses         1060       1060          
  Partials          0          0          

Powered by Codecov. Last update 5d31316...85b3aec

@@ -159,7 +162,7 @@ export default function (
sourceMap: !!(task.sourceMappings || task.sourceMap),
});

_.extend(task.options, taskOpts);
extend(task.options, taskOpts);
Copy link
Member

@Jessidhia Jessidhia Dec 26, 2016

Choose a reason for hiding this comment

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

Object.assign, or is it intended that taskOpts prototype keys are copied? How do you even get things in taskOpts' prototype since it comes from JSON?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if it's intended for taskOpts prototype keys to be copied, so I'm doing the safe thing here and maintaining the exact same behavior. If this PR is merged, then I'll do another pass at removing some of these utils like in #5043.

let cloneDeep = require("lodash/cloneDeep");
let traverse = require("../lib").default;
let assert = require("assert");
let parse = require("babylon").parse;
Copy link
Member

Choose a reason for hiding this comment

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

These could all be const

Copy link
Member Author

Choose a reason for hiding this comment

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

I also could've used arrow functions for the forEach's but I didn't because that's not what this PR is about.

@loganfsmyth loganfsmyth merged commit 1ab58d6 into babel:master Jan 15, 2017
@existentialism existentialism added the PR: Internal 🏠 A type of pull request used for our changelog categories label Jan 16, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants