Allowing a render function to return JSX and console.error a message. (…

…shakacode#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

.prettierrc

@@ -8,6 +8,9 @@ bracketSpacing: true
 jsxBracketSameLine: false
 parser: flow
 
+extends:
+- tslint-config-prettier
+
 overrides:
 - files: "*.@(css|scss)"
   options:

README.md

@@ -238,16 +238,17 @@ Another way is to use a separate webpack configuration file that can use a diffe
 
 For details on techniques to use different code for client and server rendering, see: [How to use different versions of a file for client and server rendering](https://forum.shakacode.com/t/how-to-use-different-versions-of-a-file-for-client-and-server-rendering/1352). (_Requires creating a free account._)
 
-## Specifying Your React Components: Direct or render functions
+## Specifying Your React Components: Register directly or use render-functions
 
-You have two ways to specify your React components. You can either register the React component (either function or class component) directly, or you can create a function that returns a React component, which we using the name of a "render function". Creating a function has the following benefits:
+You have two ways to specify your React components. You can either register the React component (either function or class component) directly, or you can create a function that returns a React component, which we using the name of a "render-function". Creating a render-function allows:
 
 1. You have access to the `railsContext`. See documentation for the railsContext in terms of why you might need it. You **need** a render function to access the `railsContext`.
 2. You can use the passed-in props to initialize a redux store or set up react-router.
 3. You can return different components depending on what's in the props.
 
-Note, the return value of a **render function** should be JSX or an HTML string. Do not return a
-function.
+Note, the return value of a **render function** should be either a React Function or Class Component, or an object representing server rendering results.
+
+Do not return a React Element (JSX).
 
 ReactOnRails will automatically detect a registered render function by the fact that the function takes
 more than 1 parameter. In other words, if you want the ability to provide a function that returns the
@@ -257,7 +258,7 @@ If you're not using this parameter, declare your function with the unused param:
 ```js
 const MyComponentGenerator = (props, _railsContext) => {
   if (props.print) {
-    // Wrap in a function so this is a React Component    
+    // This is a React FunctionComponent because it is wrapped in a function.
     return () => <H1>{JSON.stringify(props)}</H1>;
   }
 }

docs/basics/render-functions-and-railscontext.md

@@ -27,7 +27,7 @@ const MyAppComponent = (props, railsContext) => (
 
   // the props get passed again, but we ignore since we use a closure
   // or should we
-  (_props) =>
+  () =>
       <div>
         <p>props are: {JSON.stringify(props)}</p>
         <p>railsContext is: {JSON.stringify(railsContext)}
@@ -140,13 +140,12 @@ export default (props, railsContext) => {
 
 There's no reason that the railsContext would ever get passed to your React component unless the value is explicitly put into the props used for rendering. If you create a react component, rather than a render-function, for use by React on Rails, then you get whatever props are passed in from the view helper, which **does not include the Rails Context**. It's trivial to wrap your component in a "render-function" to return a new component that takes both:
 
-POSSIBLE ENHANCEMENT: Would it be better to offer the ability to send props this way if a flag is passed in
-the `react_component` helper? Probably not, since this is so easily done.
-
 ```js
 import React from 'react';
 import AppComponent from './AppComponent';
 const AppComponentWithRailsContext = (props, railsContext) => (
+  // Create a React Function Component so you can
+  // use the React Hooks API in this React Function Component
   () => <AppComponent {...{...props, railsContext}}/>
 )
 export default AppComponentWithRailsContext;

lib/react_on_rails/utils.rb

@@ -70,11 +70,8 @@ def self.server_bundle_js_file_path
       # Either:
       # 1. Using same bundle for both server and client, so server bundle will be hashed in manifest
       # 2. Using a different bundle (different Webpack config), so file is not hashed, and
-      #    bundle_js_path will throw.
-      # 3. Not using webpacker, and bundle_js_path always returns
-
-      # Note, server bundle should not be in the manifest
-      # If using webpacker gem per https://github.com/rails/webpacker/issues/571
+      #    bundle_js_path will throw so the default path is used without a hash.
+      # 3. Not using webpacker, and this method returns the bundle_js_file_path
       return @server_bundle_path if @server_bundle_path && !Rails.env.development?
 
       bundle_name = ReactOnRails.configuration.server_bundle_js_file

lib/react_on_rails/webpacker_utils.rb

@@ -20,6 +20,10 @@ def self.bundle_js_uri_from_webpacker(bundle_name)
       # Next line will throw if the file or manifest does not exist
       hashed_bundle_name = Webpacker.manifest.lookup!(bundle_name)
 
+      # If someday we add support for hashing the server-bundle and having that built
+      # by a webpack watch process and not served by the webpack-dev-server, then we
+      # need to add an extra config value "same_bundle_for_client_and_server" where a value of false
+      # would mean that the bundle is created by a separate webpack watch process.
       if Webpacker.dev_server.running?
         "#{Webpacker.dev_server.protocol}://#{Webpacker.dev_server.host_with_port}#{hashed_bundle_name}"
       else

node_package/src/createReactOutput.ts

@@ -45,13 +45,20 @@ export default function createReactOutput({
       // We just return at this point, because calling function knows how to handle this case and
       // we can't call React.createElement with this type of Object.
       return (renderFunctionResult as ServerRenderResult);
-    } // else we'll be calling React.createElement
-    // TODO: Can we detect if this is a React Element or a React Function Component?
-    // If already a ReactElement, then just return it.
-    // If a component, then wrap in an element
+    }
 
-    const reactComponent = renderFunctionResult as ReactComponent;
+    if (React.isValidElement(renderFunctionResult)) {
+      // If already a ReactElement, then just return it.
+      console.error(
+`Warning: ReactOnRails: Your registered render-function (ReactOnRails.register) for ${name}
+incorrectly returned a React Element (JSX). Instead, return a React Function Component by
+wrapping your JSX in a function. ReactOnRails v13 will error on this, as React Hooks do not
+work if you rerturn JSX.`);
+      return renderFunctionResult;
+    }
 
+    // If a component, then wrap in an element
+    const reactComponent = renderFunctionResult as ReactComponent;
     return React.createElement(reactComponent, props);
   }
   // else

package-scripts.yml

@@ -17,13 +17,13 @@ scripts:
   format:
     default:
       description: Format files using prettier.
-      script: concurrently --prefix "[{name}]" --names "js,json" -c "yellow,magenta,green" "nps format.js" "nps format.json"
+      script: concurrently --prefix "[{name}]" --names "ts,js,json" -c "yellow,magenta,green" "nps format.js" "nps format.json"
     listDifferent:
       description: Check that all files were formatted using prettier.
       script: |
         concurrently \
           --prefix "[{name}]" \
-          --names "js,json" \
+          --names "ts,js,json" \
           -c "yellow,magenta" \
           "nps format.js.listDifferent" \
           "nps format.json.listDifferent"

package.json

@@ -44,6 +44,7 @@
     "redux": "^4.0.1",
     "release-it": "^8.2.0",
     "ts-jest": "^25.2.1",
+    "tslint-config-prettier": "^1.18.0",
     "typescript": "^3.8.3",
     "webpack": "^3.4.1",
     "webpack-manifest-plugin": "^1.2.1"

spec/dummy/app/views/pages/context_function_return_jsx.html.erb

@@ -1,6 +1,8 @@
-<%= react_component("ContextFunctionReturnInvalidJSX", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %>
+<%= react_component("ContextFunctionReturnInvalidJSX", props: @app_props_hello, prerender: false, trace: true, id: "HelloWorld-react-component-0") %>
 <hr/>
 
-<h1>Example of error from failing to wrap the result in a function.</h1>
+<h1>Example of Console.error from failing to wrap the result in a function.</h1>
 
-See console log for expected warning and errors.
+See console log for expected error.
+
+Note, the component still rendered. In the next version, this should just error.

spec/dummy/app/views/shared/_header.erb

@@ -99,7 +99,7 @@
     <%= link_to "Server Render With Timout", server_render_with_timeout_path %>
   </li>
   <li>
-    <%= link_to "Incorrectly returning JSX rather than function", context_function_return_jsx_path %>
+    <%= link_to "Incorrectly returning React Element (JSX) rather than React Function Component", context_function_return_jsx_path %>
   </li>
   <li>
     <%= link_to "Incorrectly wrapping a pure component in a function", pure_component_wrapped_in_function_path %>

spec/dummy/client/app/components/ContextFunctionReturnInvalidJSX.jsx

@@ -5,7 +5,7 @@ import RailsContext from './RailsContext';
 
 const ContextFunctionReturnInvalidJSX = (props, railsContext) => (
   <>
-    <h3 className={css.brightColor}>Hello, {props.name}!</h3>
+    <h3 className={css.brightColor}>Hello, {props.helloWorldData.name}!</h3>
     <p>Rails Context :</p>
     <RailsContext {...{ railsContext }} />
   </>

spec/dummy/client/app/packs/clientRegistration.jsx

@@ -61,7 +61,7 @@ ReactOnRails.register({
   SetTimeoutLoggingApp,
   HelloWorldHooks,
   HelloWorldHooksContext,
-  ContextFunctionReturnJSX: ContextFunctionReturnInvalidJSX,
+  ContextFunctionReturnInvalidJSX,
   PureComponentWrappedInFunction,
 });
 

yarn.lock

@@ -9111,6 +9111,11 @@ tslib@^1.8.1, tslib@^1.9.0:
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.11.1.tgz#eb15d128827fbee2841549e171f45ed338ac7e35"
   integrity sha512-aZW88SY8kQbU7gpV19lN24LtXh/yD4ZZg6qieAJDDg+YBsJcSmLGK9QpnUjAKVG/xefmvJGd1WUmfpT/g6AJGA==
 
+tslint-config-prettier@^1.18.0:
+  version "1.18.0"
+  resolved "https://registry.yarnpkg.com/tslint-config-prettier/-/tslint-config-prettier-1.18.0.tgz#75f140bde947d35d8f0d238e0ebf809d64592c37"
+  integrity sha512-xPw9PgNPLG3iKRxmK7DWr+Ea/SzrvfHtjFt5LBl61gk2UBG/DB9kCXRjv+xyIU1rUtnayLeMUVJBcMX8Z17nDg==
+
 tsutils@^3.17.1:
   version "3.17.1"
   resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-3.17.1.tgz#ed719917f11ca0dee586272b2ac49e015a2dd759"