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

[react] Allow more return values from stateless components #22034

Closed
wants to merge 1 commit into from

Conversation

LinusU
Copy link
Contributor

@LinusU LinusU commented Dec 7, 2017

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

Opening this for some early feedback since I can't figure out how to tackle my last problem.

Basically, my original problem is that I have a stateless component that returns an array (it returns the return value of React.Children.map).

When fixing this I stumbled onto another problem; it's illegal to return undefined from a render-function, or from a stateless component. Yet this is currently allowed for normal components.

I think I got most of it right now, but for some reason I'm getting the following error:

test/tsx.tsx(13,1): error TS2605: JSX element type 'ReactRenderResult' is not a constructor function for JSX elements.
  Type 'null' is not assignable to type 'ElementClass'.

whenever a stateless component is used in JSX:

<StatelessComponent />;

Any help would be much appreciated! 🍻

@dt-bot
Copy link
Member

dt-bot commented Dec 7, 2017

types/react/index.d.ts

to authors (@pspeter3 @vsiao @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @morcerf @tkrotoff @DovydasNavickas @onigoetz @richseviora @theruther4d AssureSign (account can't be detected) Microsoft (account can't be detected)). Could you review this PR?
👍 or 👎?

@typescript-bot typescript-bot added the Popular package This PR affects a popular package (as counted by NPM download counts). label Dec 7, 2017
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 7, 2017

@LinusU The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks!

const NullElement: React.SFC = () => null;

// Render methods should not be allowed to return undefined
// $ExpectError
Copy link
Contributor

Choose a reason for hiding this comment

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

I love that we can write these tests.

@vsiao
Copy link
Contributor

vsiao commented Dec 7, 2017

As for the SFC error, I think it may actually be a problem to be solved at the compiler level. There's some background in microsoft/TypeScript#5478 about how SFCs work. If @RyanCavanaugh could chime in here that would be best.

@typescript-bot typescript-bot added the Other Approved This PR was reviewed and signed-off by a community member. label Dec 8, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Jan 3, 2018

Thanks for your contribution. This PR has failing CI tests and can not be merged in at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to merge this code in, please open a new PR that has been merged and rebased with the master branch.

@mhegazy mhegazy closed this Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Other Approved This PR was reviewed and signed-off by a community member. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants