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

Lodash 4 compatibility #585

Merged
merged 7 commits into from Feb 4, 2019
Merged

Lodash 4 compatibility #585

merged 7 commits into from Feb 4, 2019

Conversation

jgonggrijp
Copy link
Contributor

Resolves #561. Summary of changes:

  • It is now possible to run the entire test suite with Lodash instead of Underscore by passing the --lodash option to karma start. Still runs with Underscore by default. (Tip for Yarn users: you can just do yarn test --lodash).
  • Feature detection of _.any is used to determine whether the _ factory argument is Underscore or Lodash. In the latter case, some aliases are created on _ in order to keep the code changes to a minimum.
  • Everywhere a third context argument was passed to functions like _.each and _.map, _.bind is now used instead.
  • Travis build runs both the Underscore variant and the Lodash variant of the test suite (and both pass).
  • Travis config updated to use Node version 8 instead of 4, in order to resolve compatibility issues with the force-updated npm command line program.
  • index.html updated to reflect that Lodash works, too.

Note on the CONTRIBUTING.md and the .editorconfig: these files tell me that JavaScript files should be two-space indented, but the JavaScript files I edited were already tab-indented. I decided to go with the existing practice.

@bpatram bpatram merged commit 4e2f5c0 into PaulUithol:master Feb 4, 2019
@jgonggrijp
Copy link
Contributor Author

@bpatram Thanks for merging. Are you planning to make a new release from the master branch?

After submitting my pull request, I noticed that you asked somebody else to also submit their pull request to the next branch. Would it be useful if I did the same with this set of changes?

@bpatram
Copy link
Collaborator

bpatram commented Feb 4, 2019

@jgonggrijp if you have the availability to submit a PR against next then please do! I will most likely cherrypick some of the changes in that other PR so we can remove that lodash shim you added here (I believe the other PR made those conversions, but I may be mistaken)

if (!_.any) { // We have Lodash, make it imitate Underscore a bit more.
_.any = _.some;
_.all = _.every;
_.contains = _.includes;
_.pluck = _.map;
}

I'll get a new release cut within the week

@jgonggrijp
Copy link
Contributor Author

I will most likely cherrypick some of the changes in that other PR so we can remove that lodash shim you added here (I believe the other PR made those conversions, but I may be mistaken)

Backbone-relational/backbone-relational.js

If by "that other PR" you mean #574/#577, please don't cherrypick those changes. @drphelps actually replaced the Underscore methods by Lodash methods instead of aliasing them, which breaks compatibility with Underscore. I'm open to other approaches to my shim, but please let us preserve support for Underscore.

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.

Doesn't work with lodash 4.1.0
2 participants