Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

setState within componentWillMount uses internal property update #2149

Closed

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Jun 21, 2018

Release notes: none

As we're updating state on a final object, in this case state, ensure we use hardModifyReactObjectPropertyBinding. Unblocks #2137.

@@ -218,16 +224,16 @@ export function createClassInstanceForFirstRenderOnly(
}
if (stateToUpdate instanceof ObjectValue) {
let newState = new ObjectValue(realm, realm.intrinsics.ObjectPrototype);
newState.makeFinal();
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we make it final and then immediately do an Object.assign to it? Seems inconsistent. I thought finals are treated as immutable.

Should we do this after it's fully initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you beat me to this, I changed it in my latest commit.

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

OK.

I found it confusing it was even possible to make something final and then Object.assign on it. I'd expect that to fail.

Seems good overall given strategy described in #2138

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hermanventer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

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

Successfully merging this pull request may close these issues.

3 participants