Skip to content
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

Enable module pattern. #6008

Merged
merged 1 commit into from
Feb 12, 2016
Merged

Enable module pattern. #6008

merged 1 commit into from
Feb 12, 2016

Conversation

jimfb
Copy link
Contributor

@jimfb jimfb commented Feb 9, 2016

@spicyj Your Masterwork From Distant Lands, plus a couple minor bug fixes.

@gaearon
Copy link
Collaborator

gaearon commented Feb 9, 2016

It was working before, wasn’t it? Did something in the recent PRs cause it to break?

@sophiebits
Copy link
Collaborator

(Masterwork From Distant Lands is the default title for a paste in Phabricator.)

@sophiebits
Copy link
Collaborator

@gaearon Yes, it was incorrectly removed in #5884.

@jimfb
Copy link
Contributor Author

jimfb commented Feb 9, 2016

@gaearon Yes, I accidentally broke it when enabling null from stateless components

@jimfb jimfb added this to the 0.15 milestone Feb 9, 2016
inst === null ||
inst === false ||
ReactElement.isValidElement(inst),
'%s(...): Stateless function components must return a valid ReactElement.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have more consistent wording here and in _renderValidatedComponent? (Looks like that one's currently wrong because it says component instead of element.) Maybe:

A valid React element (or null) must be returned. You may have returned undefined, an array or some other invalid object.

@sophiebits
Copy link
Collaborator

I think this is good. cc @sebmarkbage to verify we want to duck type the return value about whether it has a .render property or not.

@facebook-github-bot
Copy link

@jimfb updated the pull request.

@facebook-github-bot
Copy link

@jimfb updated the pull request.

jimfb added a commit that referenced this pull request Feb 12, 2016
@jimfb jimfb merged commit 293dc75 into facebook:master Feb 12, 2016
gaearon added a commit to gaearon/react that referenced this pull request Dec 28, 2016
We have an invariant that checks the same case right afterwards.

The warning was originally added in facebook#5884 with a distinct wording.

However it was later changed to the same wording as the invariant in facebook#6008.

I don't see why we would want to have both since they're saying the same thing and with (almost) the same internal stack.
acdlite pushed a commit to acdlite/react that referenced this pull request Jan 12, 2017
We have an invariant that checks the same case right afterwards.

The warning was originally added in facebook#5884 with a distinct wording.

However it was later changed to the same wording as the invariant in facebook#6008.

I don't see why we would want to have both since they're saying the same thing and with (almost) the same internal stack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants