Skip to content

Pass incomingState to _buildState #132

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

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

berickson1
Copy link
Collaborator

This will handle the case where _buildState relies on a value that can be changed by calling setState
Fixes #131

This will handle the case where _buildState relies on a value that can be changed by calling setState
Fixes microsoft#131
@berickson1 berickson1 requested a review from deregtd November 7, 2019 05:52
@msftclas
Copy link

msftclas commented Nov 7, 2019

CLA assistant check
All CLA requirements met.

@berickson1 berickson1 merged commit 4fbf16b into microsoft:master Nov 7, 2019
3. When this component subscribes to any stores, this will be called whenever the subscription is triggered. This is the most common usage of subscriptions, and the usage created by autosubscriptions.

Any calls from this method to store methods decorated with `@autoSubscribe` will establish an autosubscription.

React’s `setState` method should not be called directly in this function. Instead, the new state should be returned and `ComponentBase` will handle the update of the state.

Note: If your _buildState ever relies on component state, utilize the incomingState argument, otherwise you risk using an old snapshot of the state (See https://github.com/microsoft/ReSub/issues/131 for more details)
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this might be noted here - https://github.com/microsoft/ReSub/blob/master/README.md#L9 - when I do breaking changes I usually have a separate migration page linked in changelog + readme for each major version change where I give an example of what to look for, and how to change it. Otherwise I get bombed with issues later - pay me now / pay me later. $0.02

Copy link
Contributor

Choose a reason for hiding this comment

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

This is separate but possibly also worth noting I saw my react migration tidbits in there but I'm not sure they are valid anymore? 'UNSAFE_' anything - https://github.com/microsoft/ReSub/blob/master/README.md#L370

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

Successfully merging this pull request may close these issues.

ReSub v2 setState not working?
4 participants