Deprecate React.addons.update #2909

Closed
zpao opened this Issue Jan 22, 2015 · 8 comments

Comments

Projects
None yet
7 participants
@zpao
Member

zpao commented Jan 22, 2015

This is the 2nd easiest addon to get rid of.

see also #680

TODO:

  • figure out where shared dependencies (keyOf, invariant, Object.assign) live
  • new name
  • new repo
  • file existing issues in new repo
  • update internal usage @ FB
@graue

This comment has been minimized.

Show comment
Hide comment
@graue

graue Jan 5, 2016

Contributor

Sorry about the dupe. Maybe as the first subtask here, we can de-emphasize this addon in the documentation?

My thinking is to rewrite https://facebook.github.io/react/docs/update.html to first suggest using array spread/object spread syntax ($push is easily replaced with array spread, btw 🙂). We now tell people to use Babel right in the getting started guide, so suggesting a feature that needs transpilation seems reasonable.

Second, mention immutable-js for more advanced scenarios where you need to make deeply-nested updates. In my experience well-written React code rarely needs to update state nested more than one level deep, so I disagree with the current doc's implication that such deep nesting is common or natural (what do you think?).

Then, only lastly, document react-addons-update with a note explaining that it's a historic artifact from before immutable-js existed, and may be removed from a future version of React.

Contributor

graue commented Jan 5, 2016

Sorry about the dupe. Maybe as the first subtask here, we can de-emphasize this addon in the documentation?

My thinking is to rewrite https://facebook.github.io/react/docs/update.html to first suggest using array spread/object spread syntax ($push is easily replaced with array spread, btw 🙂). We now tell people to use Babel right in the getting started guide, so suggesting a feature that needs transpilation seems reasonable.

Second, mention immutable-js for more advanced scenarios where you need to make deeply-nested updates. In my experience well-written React code rarely needs to update state nested more than one level deep, so I disagree with the current doc's implication that such deep nesting is common or natural (what do you think?).

Then, only lastly, document react-addons-update with a note explaining that it's a historic artifact from before immutable-js existed, and may be removed from a future version of React.

@brigand

This comment has been minimized.

Show comment
Hide comment
@brigand

brigand Jan 5, 2016

Contributor

Immutable.js and the update addon are different. With the update addon you keep plain js objects, while immutable.js has custom wrapper objects. The update addon is useful for components where you don't want to make assumptions, while immutable.js is good for situations where you can make assumptions. They're both valid tools to use.

Contributor

brigand commented Jan 5, 2016

Immutable.js and the update addon are different. With the update addon you keep plain js objects, while immutable.js has custom wrapper objects. The update addon is useful for components where you don't want to make assumptions, while immutable.js is good for situations where you can make assumptions. They're both valid tools to use.

@graue

This comment has been minimized.

Show comment
Hide comment
@graue

graue Jan 6, 2016

Contributor

Yup, I'm aware, and that's why I suggested array/object spread as the recommended replacement for most people. It too sticks to plain JavaScript data structures, but has a simpler API. See my remarks in #5780.

Contributor

graue commented Jan 6, 2016

Yup, I'm aware, and that's why I suggested array/object spread as the recommended replacement for most people. It too sticks to plain JavaScript data structures, but has a simpler API. See my remarks in #5780.

@jmm

This comment has been minimized.

Show comment
Hide comment
@jmm

jmm Feb 4, 2016

@graue

My thinking is to rewrite https://facebook.github.io/react/docs/update.html to first suggest using array spread/object spread syntax [...] suggesting a feature that needs transpilation seems reasonable.

As I mentioned in #5780 there's some risk involved in that that makes it worth considering whether you want to encourage people to do it, and tip them off about the risk.

In my experience well-written React code rarely needs to update state nested more than one level deep, so I disagree with the current doc's implication that such deep nesting is common or natural (what do you think?).

FWIW it's probably more common with something like Redux, which mentions using this helper. (Obviously what other libraries are doing is beside the point of what you want to ship / document here, and the helper will be available as an independent package anyway.)


Is the new name item here covered now by react-addons-update?

jmm commented Feb 4, 2016

@graue

My thinking is to rewrite https://facebook.github.io/react/docs/update.html to first suggest using array spread/object spread syntax [...] suggesting a feature that needs transpilation seems reasonable.

As I mentioned in #5780 there's some risk involved in that that makes it worth considering whether you want to encourage people to do it, and tip them off about the risk.

In my experience well-written React code rarely needs to update state nested more than one level deep, so I disagree with the current doc's implication that such deep nesting is common or natural (what do you think?).

FWIW it's probably more common with something like Redux, which mentions using this helper. (Obviously what other libraries are doing is beside the point of what you want to ship / document here, and the helper will be available as an independent package anyway.)


Is the new name item here covered now by react-addons-update?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 1, 2016

Member

This might be helpful if we decide to rewrite the doc to use the spread operator: http://redux.js.org/docs/recipes/UsingObjectSpreadOperator.html

Member

gaearon commented Apr 1, 2016

This might be helpful if we decide to rewrite the doc to use the spread operator: http://redux.js.org/docs/recipes/UsingObjectSpreadOperator.html

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 1, 2016

Member

Also if @kolodny is interested we can just direct people to his repo: https://github.com/kolodny/immutability-helper

Member

gaearon commented Apr 1, 2016

Also if @kolodny is interested we can just direct people to his repo: https://github.com/kolodny/immutability-helper

@seansfkelley seansfkelley referenced this issue in felixgirault/pure-render-decorator Apr 15, 2016

Merged

Replace react-addons-shallow-compare with fbjs. #3

@ben-foxmoore ben-foxmoore referenced this issue in react-toolbox/react-toolbox Aug 15, 2016

Closed

Error: Cannot resolve module 'react-addons-update' #695

@boxofrox

This comment has been minimized.

Show comment
Hide comment
@boxofrox

boxofrox Feb 11, 2017

Food for thought.

If you create a react-community organization, you could extract addons and the like into repos there, and stipulate that react-community projects are not actively developed or used by Facebook, but PRs and bugfixes from the community are still welcome. Elm uses this method to extend the functionality of its core libraries when it's not certain said functionality belongs in the core.

If you do fork react-addons-update into a new repo, I nominate persistent-update for the project name. The idea being that the original object persists after the update. It doesn't appear to be used on npmjs.org.

boxofrox commented Feb 11, 2017

Food for thought.

If you create a react-community organization, you could extract addons and the like into repos there, and stipulate that react-community projects are not actively developed or used by Facebook, but PRs and bugfixes from the community are still welcome. Elm uses this method to extend the functionality of its core libraries when it's not certain said functionality belongs in the core.

If you do fork react-addons-update into a new repo, I nominate persistent-update for the project name. The idea being that the original object persists after the update. It doesn't appear to be used on npmjs.org.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Apr 21, 2017

Member

I figure we’re done here.

Member

gaearon commented Apr 21, 2017

I figure we’re done here.

@gaearon gaearon closed this Apr 21, 2017

@jsonz1993 jsonz1993 referenced this issue in shigebeyond/react-native-sk-countdown Jun 3, 2017

Open

Cannot read property 'update' of undefined 问题解决 #12

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