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

Make ListSelections immutable #1421

Merged
merged 6 commits into from Apr 26, 2018

Conversation

Projects
None yet
1 participant
@smashwilson
Member

smashwilson commented Apr 25, 2018

To lay some groundwork for converting StagingView to React, I'm converting our ListSelection models to be immutable.

Rather than updating the selection in-place:

const selection = new ListSelection({items: ['a', 'b', 'c'});
selection.selectFirstItem();

Mutation methods return a new ListSelection with the modification made:

const selection0 = new ListSelection({items: ['a', 'b', 'c']);
const selection1 = selection0.selectFirstItem();

This should make manipulating selection models in setState() calls safer and more ergonomic, because we can use its updater method variant:

prevState is a reference to the previous state. It should not be directly mutated. Instead, changes should be represented by building a new object based on the input from prevState and props.

actionMethod = () => {
  this.setState(prevState => ({
    theSelection: prevState.theSelection.selectFirstItem(), 
  }));
}
  • Make ListSelection immutable
  • Make CompositeListSelection immutable
  • Update the StagingView to use its CompositeListSelection immutably

smashwilson added some commits Apr 20, 2018

@smashwilson

This comment has been minimized.

Member

smashwilson commented Apr 25, 2018

By the way, I did this for FilePatchSelection a long time ago when I converted the FilePatchController to React. So right now one of our selection models is immutable and the others are mutable, which bugs me. 😄

smashwilson added some commits Apr 25, 2018

@smashwilson

This comment has been minimized.

Member

smashwilson commented Apr 26, 2018

:shipit: to prepare for the final phase of the React port 🤘

@smashwilson smashwilson merged commit 902b511 into master Apr 26, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@smashwilson smashwilson deleted the aw/immutable-selections branch Apr 26, 2018

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