Skip to content

Conversation

clayallsopp
Copy link
Contributor

We had a logic bug in our app where replaceProps was called before the component was renderComponent'd. The current error thrown is Uncaught TypeError: Cannot read property 'constructor' of undefined, which isn't super helpful.

Not sure this is the best/correct way to do this, but would love to know what is :)

@zpao
Copy link
Member

zpao commented Aug 28, 2013

I think this might be better protected against by inspecting lifecycle state. We do something similar for replaceState in composite components...

And now that I'm looking more, we actually protect against this in receiveProps but setProps doesn't go through that path. I think we could use some consolidation but for the time being, let's put an invariant into replaceProps that does exactly what receiveProps does. Give that a try and see if it fixes your case! (and if you wrote a test to ensure you get that error, that would be cool too 😉)

@clayallsopp
Copy link
Contributor Author

@zpao updated, with a test

@zpao
Copy link
Member

zpao commented Sep 3, 2013

👍 I'm pulling this in for review internally right now

var instance =
<PropsToUpdate
value="hello"
/>;
Copy link
Member

Choose a reason for hiding this comment

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

We can fit this whole var instance = .... onto a single line, so lets do that.

@clayallsopp
Copy link
Contributor Author

Cool, done! Any reason grunt lint doesn't pick up the 80 char limit?

@zpao
Copy link
Member

zpao commented Sep 5, 2013

It's a soft limit but we try to stick to it as much as possible. It would be great to make it possible to have multiple passes so that we could give a warning but not fail (the maxlen option results in errors if we turn it on). Internally we have the tooling set up to warn when linting but not block commits (unlike other lint errors).

@zpao
Copy link
Member

zpao commented Sep 5, 2013

Accepted. Thanks a lot @clayallsopp!

zpao added a commit that referenced this pull request Sep 5, 2013
More helpful message if you update an unrendered component
@zpao zpao merged commit 4f0dea3 into facebook:master Sep 5, 2013
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.

2 participants