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

#2325 : Add $inc support for Helpers update #2352

Closed
wants to merge 1 commit into from
Closed

#2325 : Add $inc support for Helpers update #2352

wants to merge 1 commit into from

Conversation

ThomasCrevoisier
Copy link
Contributor

No description provided.

@ThomasCrevoisier ThomasCrevoisier changed the title #2325 : Add support for Helpers update #2325 : Add $inc support for Helpers update Oct 15, 2014
@syranide
Copy link
Contributor

As far as I understand update it looks great. AFAIK the intention was to imitate the MongoDB interface which does support inc, but I'm not sure if it's really necessary in React as you can just add to the value directly (obviously a bit more verbose though) and update is intended to just be a really simple helper.

That being said, big thanks for contributing, I'd love to find something of significance that you could attack... :)

@ThomasCrevoisier
Copy link
Contributor Author

I see ^^ Well, the feature was quite simple to implement, so it's no big deal. And I'd love to work on something more significant :)

@totty90
Copy link

totty90 commented Oct 23, 2014

Hy! Thanks (: This would be really helpful to be included. As I've explained here: #2325

This would allow to reuse the same code for a mongodb database as in my case. If you want to reuse the logic between client and server side this would be an amazing addition. Waiting to be included. Currently I have the same logic, but somewhere is duplicated because I need update and the $inc doesn't work yet.

For example this is the logic which runs both on client and on server:

var update = {};
job.time += 0.2;
updates.job = {$inc: {time: 0.2}};
return updates;

This issue solves this problem:

var update = {};
updates.job = {$inc: {time: 0.2}};
return updates;

// outside of that method, on the client side
update(job, updates.job);

Notice that I don't have duplicated logic, which is a pain to maintain (job.time += 0.2; and {$inc: {time: 0.2}};).

In the future more people will use this technique to predict on the client and validate/update on the server using the exact same logic. Is very DRY and less prone to errors, inconsistencies.

@syranide
Copy link
Contributor

@totty90 If your goal is to be compatible with MongoDB, I would recommend that you fork/reimplement this helper for that specific goal. That's not to say it shouldn't be added to this React addon, but AFAIK MongoDB is just an inspiration for this helper, not the goal (as far as I'm aware, it's provided just to kickstart people who are new to React).

@totty90
Copy link

totty90 commented Oct 23, 2014

@syranide Yes, I understand. But if we can both find some common use, I would appreciate. I would not like to keep a different fork of react.. And other people might even use it this way.

@syranide
Copy link
Contributor

@totty90 React.addons.* is not part of React, it's just shipped as part of the React bundle for now. The plan is to move them out into separate NPM modules when everything is figured out, wiki link. React.addons.update has no React core dependencies.

@totty90
Copy link

totty90 commented Oct 23, 2014

@syranide I know about that. Maybe at least allow to monkey patch the lib externally, at runtime, without the need of a fork could be a solution?

@chenglou
Copy link
Contributor

@petehunt

@sebmarkbage
Copy link
Collaborator

update is deprecated in favor of immutable.js

We don't want to turn this into a DSL.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2016

You might be interested in https://github.com/kolodny/immutability-helper that lets you define custom operations by name.

@facebook-github-bot
Copy link

@ThomasCrvsr updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants