search

omniscientjs / immstruct

Immutable data structures with history for top-to-bottom properties in component based libraries like React. Based on Immutable.js
374 stars 21 forks source link

Update deps #88

Closed neftaly closed 7 years ago

neftaly commented 7 years ago

Minor version bump, as there are several new features in Immutable. Cheers!

mikaelbr commented 7 years ago

Thanks for taking the time 👍

I usually don't bump minor dependencies on libraries as it would reduce possibility to dedupe dependencies. I think libraries should set the lowest supported of the newest major release of a dependency. This way you say "I support all releases newer than this" and if another library also do the same, we can use the same dependency. What are your thoughts on this?

neftaly commented 7 years ago

The problem is that the current build script bundles all libraries together and exports as one. I'd like to tackle that, but want to get everything up-to-date first.

mikaelbr commented 7 years ago

Hm. But it shouldn't include Immutable though. And I don't think it is in the current dist file? Seeing your commit, it has 5000 lines in addition, so clearly some extra stuff is included there. But this part is ment to fix that: https://github.com/omniscientjs/immstruct/blob/master/makeBundle.js#L20 and https://github.com/omniscientjs/immstruct/blob/master/makeBundle.js#L24-L25

Maybe there's something not working correctly in the build scripts.

neftaly commented 7 years ago

Er, regarding my last comment, dist/immstruct.js isn't actually exported. My bad. That said, the several libs have had major releases, hence the code updates. I can revert the non-majors if that helps?

mikaelbr commented 7 years ago

Absolutely. I think I was unclear, most updates are actually very much welcome, and for devDependencies it adds value to always be up to date with the dependencies. What I was talking about goes for normal dependencies only. EventEmitter3 can be bumped also.

If you could bump the devDependencies and EventEmitter I'd gladly merge the commit. Also, could you avoid bumping/commiting the dist files?

neftaly commented 7 years ago

Have rebased it, is this acceptable?

mikaelbr commented 7 years ago

Good job. Thanks 👍 Not trying to be annoying, but could you not bump the package version at all? Think I was being somewhat unclear on this. But usually, library maintainers would like to do this themselves. As there might be more changes that should go into this, or due to automatic steps etc. It's very unusual for PRs to bump the version it self 😄

neftaly commented 7 years ago

Sorry, too used to chasing up my team to do it, haha.

mikaelbr commented 7 years ago

Great. Thanks 🎉