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

Replace lodash 3 #37

Merged
merged 16 commits into from Feb 28, 2016
Merged

Replace lodash 3 #37

merged 16 commits into from Feb 28, 2016

Conversation

Olical
Copy link
Owner

@Olical Olical commented Feb 23, 2016

I'm trying to remove the dependency on lodash, as discussed in #36, hopefully I can replace every function with relative ease. Thoughts?

Work in progress untill all tasks are marked as done.

Functions

  • lodash.assign
  • lodash.some
  • lodash.filter
  • lodash.clone
  • lodash.mapvalues
  • lodash.camelcase
  • Wide browser support

Bonus

  • Rename src to lib
  • Replace typeof checks with functions
  • Update remaining dependencies
  • Move lodash replacement functions into some tested util files

@Olical Olical self-assigned this Feb 23, 2016
}

function clone (value) {
return Object.assign({}, value)
Copy link

Choose a reason for hiding this comment

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

I believe this would drop browser support for IE 11 unless you polyfill it. MDN has a polyfill here, however I would just take the polyfill and assign it to your function or stick with loadash's clone. Great idea, loved your article and approach to incorporating mutative libraries with react. I think we're going to give this a shot with an upcoming project.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, you're right. I was just going for "remove dependency on lodash" first then look into browser support. This will be a major version bump too, just in case I have to drop any support for anything by accident. I'll check out the polyfills after I'm done replacing things.

I'm glad you liked it, I hope it comes in useful! I'm using it in production and I think others are too (99% sure). Let me know if you need help, or raise issues. If you do end up using it maybe I could start a list of "people who used it for things" in the readme too.

@Olical
Copy link
Owner Author

Olical commented Feb 26, 2016

Okay, I have replaced all lodash functions with <ES5 compatible code (I think? Verify this if you want!).

I've also moved stray functions lying around in Element.js into their own files and added some tests.

Thoughts? If this looks good to all of you, just drop a thumbs up in the comments or something so I know someone has seen it :)

Thanks a lot for your input!

@kitten
Copy link
Contributor

kitten commented Feb 26, 2016

@Olical As already pointed out Object.assign is the only major compatibility problem now.
Its support is kind of new and you can see the compatibility table here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign

Babel for example goes through the trouble of polyfilling helpers, like Object.assign from core.js https://github.com/zloirock/core-js

This is a bigger issue than it seems, because this means that any project that uses this library automatically doesn't support Internet Explorer and some older versions of other popular browsers.

imo it wasn't a problem to use lodash in this project. Redux uses lodash as well, after all. But this would seriously benefit from a transpiler like Babel, that removes the worries of compatibility completely.

For now this is in my opinion fine. But fewer dependencies also means that this project has to cover some helpers with all the edge cases in the future 😉

Also, something minor. I consider these two functions really unnecessary ^^

var isString = require('./utils/isString')
var isUndefined = require('./utils/isUndefined')

I hope my two cents help you out :)

@Olical
Copy link
Owner Author

Olical commented Feb 27, 2016

I completely agree about the assign thing, sorry about that, thought I'd
replaced them all! I'll replace that with a portable version. I only added
those type check functions because it saves performing string comparison
which allow spelling errors etc. If I had the full lodash to hand, I would
have used those functions anyway, I just prefer wrapping those checks
because you can use them in calls to filter for example.

I think there's a few things I'm using right now that won't work in older
browsers. What's React support like too, surely we only need to support the
browsers React supports.

Thanks for reminding me about assign though.
On 26 Feb 2016 23:54, "Phil Plückthun" notifications@github.com wrote:

@Olical https://github.com/Olical As already pointed out Object.assign
is the only major compatibility problem now.
Its support is kind of new and you can see the compatibility table here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign

Babel for example goes through the trouble of polyfilling helpers, like
Object.assign from core.js https://github.com/zloirock/core-js

This is a bigger issue than it seems, because this means that any project
that uses this library automatically doesn't support Internet Explorer and
some older versions of other popular browsers.

imo it wasn't a problem to use lodash in this project. Redux uses lodash
as well, after all. But this would seriously benefit from a transpiler like
Babel, that removes the worries of compatibility completely.

For now this is in my opinion fine. But fewer dependencies also means that
this project has to cover some helpers with all the edge cases in the
future [image: 😉]

Also, something minor. I consider these two functions really unnecessary ^^

var isString = require('./utils/isString')var isUndefined = require('./utils/isUndefined')

I hope my two cents help you out :)


Reply to this email directly or view it on GitHub
#37 (comment).

@kitten
Copy link
Contributor

kitten commented Feb 27, 2016

@Olical Yea, the only critical thing is assign. Otherwise there shouldn't be stuff that is as critical.

I do get writing functions for some type checks, but they're so ridiculously simple, that it's not worth requiring them. Normally a typeof x === 'string' is so short, that it's not necessary to have isString(x).

Stylistically, yes, it can make a small difference. But only if the function is doing more than just using one expression and one operator imho ^^ And for filter and the likes it is pretty much the same. Anonymous functions are whipped up very quickly. And people are moving towards due to arrow functions. But meh, that was just a small comment.

@Olical
Copy link
Owner Author

Olical commented Feb 27, 2016

@philpl Fixing the assign now 😄

With regards to the little type checking functions: I guess I'm used to abstracting typeof use because checking if something is an array or a plain object isn't trivial. Because those require some slightly more complex checks I also like to check all other types through functions.

If I were writing ClojureScript or something I'd be using functions too, it just feels right to me. We're talking about a very small stylising thing, you're right, but it's an interesting conversation all the same! I like everything being functions so I can compose them and throw them into other functions as predicates. With language keywords (well, in JavaScript) that can't be done.

Also, not arguing with you at all! I just find it interesting to discuss this sort of thing! Thanks for all of the comments.

@Olical
Copy link
Owner Author

Olical commented Feb 27, 2016

And yeah, I'm going to add my own assign then make clone use that.

Olical added a commit that referenced this pull request Feb 28, 2016
@Olical Olical merged commit 4f59c66 into master Feb 28, 2016
Olical added a commit that referenced this pull request Feb 28, 2016
@Olical Olical deleted the replace-lodash-3 branch June 21, 2016 18:14
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

4 participants