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

Allowing a render function to return JSX and console.error a message. #1280

Merged

Conversation

justin808
Copy link
Member

@justin808 justin808 commented May 5, 2020

to return JSX and detcting that

If you were a user of React on Rails, would you mind the small required changes to ensure correct functionality? In other words, we will always call React.createElement in the library, rather than just taking JSX from some function that took props and was invoked by React on Rails.

See the discussion here:
#1198 (comment)


This change is Reviewable

if (React.isValidElement(renderFunctionResult)) {
// If already a ReactElement, then just return it.
return renderFunctionResult;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, this skips the maybe essential call of React.createElement(component, props).

This 100% breaks if hooks are used.

Should we just error out and force the change to wrap the result JSX in a function with zero params?

or should we make the upgrade easier for 99% of the cases that did not use Hooks.

@justin808
Copy link
Member Author

If you take 2, correct is to use a React Function Component:

import React from 'react';
import AppComponent from './AppComponent';
const AppComponentWithRailsContext = (props, railsContext) => (
  // It's better to create a React Function Component so you can
  // use the React Hooks API in this React Function Component
  () => <AppComponent {...{...props, railsContext}}/>
)
export default AppComponentWithRailsContext

Currently, this works, to just return a React Element (JSX)

import React from 'react';
import AppComponent from './AppComponent';
const AppComponentWithRailsContext = (props, railsContext) => (
  <AppComponent {...{...props, railsContext}}/>
)
export default AppComponentWithRailsContext

I propose that I'll allow the second case for v12, but print a warning console message indicating that the JSX should get wrapped in a function to make the return value a React Function Component.

@justin808 justin808 force-pushed the justin808/allow-react-element-keep-backwards-compat branch from 0ec0802 to b0cca4c Compare May 6, 2020 09:26
from a render-function...

Warning clearly states you need to return a React Function Component
instead.
@justin808 justin808 force-pushed the justin808/allow-react-element-keep-backwards-compat branch from b0cca4c to 06e5106 Compare May 6, 2020 09:27
@justin808 justin808 changed the title WIP to consider allowing a render function Allowing a render function to return JSX and console.error a message. May 7, 2020
@justin808 justin808 merged commit 5b3148a into master May 7, 2020
@justin808 justin808 deleted the justin808/allow-react-element-keep-backwards-compat branch May 7, 2020 22:28
justin808 added a commit that referenced this pull request Jun 9, 2020
…#1280)

* Add console.error for invalid React Element from a render-function...

Warning clearly states you need to return a React Function Component
instead.

Docs updated
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.

1 participant