From be37b3410de1974f16066951bc41fa73fa6f3c61 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 15 Apr 2020 16:29:29 -1000 Subject: [PATCH 01/10] Major change to fix issues with React Hooks --- CHANGELOG.md | 22 ++++++++++++- CONTRIBUTING.md | 11 +++---- node_package/src/createReactElement.ts | 15 +++++++-- node_package/src/generatorFunction.ts | 8 ++++- node_package/src/types/index.d.ts | 2 +- node_package/tests/generatorFunction.test.js | 16 +++++++--- package.json | 2 +- spec/dummy/Procfile.static | 5 +-- spec/dummy/app/views/pages/index.html.erb | 14 ++++++++ .../pages/server_side_hello_world.html.erb | 2 +- .../server_side_hello_world_hooks.html.erb | 4 +++ spec/dummy/app/views/shared/_header.erb | 9 ++++-- .../client/app/components/HelloWorldHooks.jsx | 19 +++++++++++ .../app/components/HelloWorldHooksContext.jsx | 32 +++++++++++++++++++ .../client/app/packs/clientRegistration.jsx | 4 +++ .../client/app/startup/ClientRenderedHtml.jsx | 9 +++++- .../client/app/startup/serverRegistration.jsx | 4 +++ spec/dummy/config/routes.rb | 1 + spec/dummy/spec/system/integration_spec.rb | 9 ++++++ spec/dummy/yarn.lock | 29 ++++++++++------- yarn.lock | 23 +++++++------ 21 files changed, 190 insertions(+), 50 deletions(-) create mode 100644 spec/dummy/app/views/pages/server_side_hello_world_hooks.html.erb create mode 100644 spec/dummy/client/app/components/HelloWorldHooks.jsx create mode 100644 spec/dummy/client/app/components/HelloWorldHooksContext.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 1756e130c..e7358c302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,27 @@ Changes since last non-beta release. * Removal of config.symlink_non_digested_assets_regex as it's no longer needed with rails/webpacker. If any business needs this, we can move the code to a separate gem. - +### [12.0.0] +#### BREAKING CHANGE +In order to solve the issues regarding React Hooks compatability: + +Registered components may either do one of the following if functions: + +##### Correct +Either of these will work: +1. Take 2 props and return a function that takes zero or one params and returns JSX. + ```js + export default (props, _railsContext) => () => ; + ``` + +2. Take only zero or one params and you return a JSX literal + ```js + export default (props) => ; + ``` +##### Broken, as this function takes two params and it returns a JSX Literal +```js +export default (props, _railsContext) => ; +``` ### [11.3.0] - 2019-05-24 #### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f9b20f9b4..3d86ecada 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -128,15 +128,12 @@ _Note: running `npm i` automatically builds the npm package before installing. H After checking out the repo, making sure you have rvm and nvm setup (setup ruby and node), cd to `spec/dummy` and run `bin/setup` to install ruby dependencies. You can also run `bin/console` for an interactive prompt that will allow you to experiment. ### Local Node Package -Because the example and dummy apps rely on the react-on-rails node package, they should link directly to your local version to pick up any changes you may have made to that package. To achieve this, switch to the dummy app's root directory and run this command below which runs something like [this script](spec/dummy/package.json#L14) +The default setup of spec/dummy uses a yarn link to the `/node_package/lib` directory. To test changes +to the node module of react-on-rails, run this from the top directory `yarn run build-watch`. -```sh -cd react_on_rails/spec/dummy -yarn run install-react-on-rails -``` -_Note: this runs npm under the hood as explained in **Test NPM for react-on-rails** section above_ +Then you just need to run `yarn` in the `spec/dummy` directory. -From now on, the example and dummy apps will use your local node_package folder as the react-on-rails node package. This will also be done automatically for you via the `rake examples:gen_all` rake task. +Note, the example and dummy apps will use your local node_package folder as the react-on-rails node package. This will also be done automatically for you via the `rake examples:gen_all` rake task. *Side note: It's critical to use the alias section of the webpack config to avoid a double inclusion error. This has already been done for you in the example and dummy apps, but for reference:* diff --git a/node_package/src/createReactElement.ts b/node_package/src/createReactElement.ts index fe641c4b8..409845018 100644 --- a/node_package/src/createReactElement.ts +++ b/node_package/src/createReactElement.ts @@ -2,6 +2,7 @@ import React from 'react'; import type { CreateParams, ComponentVariant, RenderFunction, CREReturnTypes } from './types/index'; +import isRouterResult from "./isCreateReactElementResultNonReactComponent"; /** * Logic to either call the generatorFunction or call React.createElement to get the @@ -36,9 +37,19 @@ export default function createReactElement({ } } + // TODO: replace any + let ReactComponent: any; if (generatorFunction) { - return (component as RenderFunction)(props, railsContext); + // Let's invoke the function to get the result + ReactComponent = (component as RenderFunction)(props, railsContext); + if (isRouterResult(ReactComponent)) { + // 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 ReactComponent; + } // else we'll be calling React.createElement + } else { + ReactComponent = component; } - return React.createElement(component as ComponentVariant, props); + return React.createElement(ReactComponent as ComponentVariant, props); } diff --git a/node_package/src/generatorFunction.ts b/node_package/src/generatorFunction.ts index ff7b4d41b..ed1619bea 100644 --- a/node_package/src/generatorFunction.ts +++ b/node_package/src/generatorFunction.ts @@ -1,6 +1,6 @@ // See discussion: // https://discuss.reactjs.org/t/how-to-determine-if-js-object-is-react-component/2825/2 -import { ComponentOrRenderFunction } from './types/index'; +import { ComponentOrRenderFunction, RenderFunction } from "./types/index"; /** * Used to determine we'll call be calling React.createElement on the component of if this is a @@ -13,6 +13,12 @@ export default function generatorFunction(component: ComponentOrRenderFunction): return false; } + // If zero or one args, then we know that this is a regular function that will + // return a React component + if (component.length < 2) { + return false; + } + // es5 or es6 React Component return !component.prototype.isReactComponent; } diff --git a/node_package/src/types/index.d.ts b/node_package/src/types/index.d.ts index 8abb5abee..690e13ca8 100644 --- a/node_package/src/types/index.d.ts +++ b/node_package/src/types/index.d.ts @@ -46,7 +46,7 @@ type AuthenticityHeaders = {[id: string]: string} & {'X-CSRF-Token': string | nu type StoreGenerator = (props: {}, railsContext: RailsContext) => Store -type CREReturnTypes = {renderedHtml: string} | {redirectLocation: {pathname: string; search: string}} | {routeError: Error} | {error: Error} | ReactElement; +type CREReturnTypes = {renderedHtml: string} | {redirectLocation: {pathname: string; search: string}} | {routeError: Error} | {error: Error} | ReactElement | React.FC; export type { // eslint-disable-line import/prefer-default-export ComponentOrRenderFunction, diff --git a/node_package/tests/generatorFunction.test.js b/node_package/tests/generatorFunction.test.js index 60c6b33b4..4224362f0 100644 --- a/node_package/tests/generatorFunction.test.js +++ b/node_package/tests/generatorFunction.test.js @@ -52,17 +52,25 @@ describe('generatorFunction', () => { expect(generatorFunction(ES6ComponentChild)).toBe(false); }); - it('returns true for a stateless functional component', () => { + it('returns false for a stateless functional component with zero params', () => { + expect.assertions(1); + + const pureComponent = () =>

Hello

; + + expect(generatorFunction(pureComponent)).toBe(false); + }); + + it('returns false for a stateless functional component with one param', () => { expect.assertions(1); /* eslint-disable react/prop-types */ const pureComponent = (props) =>

{props.title}

; /* eslint-enable react/prop-types */ - expect(generatorFunction(pureComponent)).toBe(true); + expect(generatorFunction(pureComponent)).toBe(false); }); - it('returns true for a generator function', () => { + it('returns true for a generator function (containing two params)', () => { expect.assertions(1); const foobarComponent = createReactClass({ @@ -71,7 +79,7 @@ describe('generatorFunction', () => { }, }); - const foobarGeneratorFunction = () => foobarComponent; + const foobarGeneratorFunction = (_props, _railsContext) => foobarComponent; expect(generatorFunction(foobarGeneratorFunction)).toBe(true); }); diff --git a/package.json b/package.json index 706272b45..a7e05f854 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "prepare": "yarn run build", "prepublish": "npm run prepare", "build": "yarn run clean && yarn run tsc", - "build-watch": "babel --watch --out-dir node_package/lib node_package/src", + "build-watch": "yarn run clean && yarn run tsc --watch", "lint": "eslint --ext '.js,.ts' .", "check": "yarn run lint && yarn run test && yarn run type-check", "type-check": "yarn run tsc --noEmit --noErrorTruncation", diff --git a/spec/dummy/Procfile.static b/spec/dummy/Procfile.static index 6493bdcb8..380bcdee2 100644 --- a/spec/dummy/Procfile.static +++ b/spec/dummy/Procfile.static @@ -5,7 +5,4 @@ rails: rails s -b 0.0.0.0 # Build client assets, watching for changes. -rails-client-assets: sh -c 'rm -rf public/webpack/development/*' || true && sh -c 'yarn run build:dev:client' - -# Build server assets, watching for changes. Remove if not server rendering. -rails-server-assets: sh -c 'yarn run build:dev:server' +rails-client-assets: sh -c 'rm -rf public/webpack/development/*' || true && sh -c 'yarn run build:dev:watch' diff --git a/spec/dummy/app/views/pages/index.html.erb b/spec/dummy/app/views/pages/index.html.erb index c96d54a87..9f32b2813 100644 --- a/spec/dummy/app/views/pages/index.html.erb +++ b/spec/dummy/app/views/pages/index.html.erb @@ -42,6 +42,20 @@ This page demonstrates a few things the other pages do not show: <%= react_component("HelloWorld", props: @app_props_server_render, trace: true, id: "HelloWorld-react-component-1") %>
+

React Rails Server Rendering with Hooks

+
+    <%%= react_component("HelloWorldHooks", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-6") %>
+
+<%= react_component("HelloWorldHooks", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-6") %> +
+ +

React Rails Server Rendering with Hooks with rails_context

+
+    <%%= react_component("HelloWorldHooksContext", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-7") %>
+
+<%= react_component("HelloWorldHooksContext", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-7") %> +
+

Simple Client Rendered Component

diff --git a/spec/dummy/app/views/pages/server_side_hello_world.html.erb b/spec/dummy/app/views/pages/server_side_hello_world.html.erb
index afa3fe761..8e8b51e32 100644
--- a/spec/dummy/app/views/pages/server_side_hello_world.html.erb
+++ b/spec/dummy/app/views/pages/server_side_hello_world.html.erb
@@ -6,7 +6,7 @@
 

This example demonstrates server side rendering. The only difference from client rendering is that the prerender option is true.

- The source HTML of this page will only show a DIV with an ID matching HelloWorld.
+ The source HTML of this page will only show a DIV with server rendering tags.

     <%= '
' %> <%= '

Hello, ' %> diff --git a/spec/dummy/app/views/pages/server_side_hello_world_hooks.html.erb b/spec/dummy/app/views/pages/server_side_hello_world_hooks.html.erb new file mode 100644 index 000000000..c46351d10 --- /dev/null +++ b/spec/dummy/app/views/pages/server_side_hello_world_hooks.html.erb @@ -0,0 +1,4 @@ +<%= react_component("HelloWorldHooks", props: @app_props_server_render, prerender: true, trace: true, id: "HelloWorld-react-component-0") %> +
+ +

React Rails Server Rendering with Hooks

diff --git a/spec/dummy/app/views/shared/_header.erb b/spec/dummy/app/views/shared/_header.erb index 39fa1a1b3..407f7837d 100644 --- a/spec/dummy/app/views/shared/_header.erb +++ b/spec/dummy/app/views/shared/_header.erb @@ -1,4 +1,7 @@
    +
  • + <%= link_to "One Page with Many Examples at Once", root_path %> +
  • <%= link_to "Hello World Component Client Rendered", client_side_hello_world_path %>
  • @@ -29,6 +32,9 @@
  • <%= link_to "Hello World Component Server Rendered", server_side_hello_world_path %>
  • +
  • + <%= link_to "Hello World Component Server Rendered Hooks", server_side_hello_world_hooks_path %> +
  • <%= link_to "Hello World Component Server Rendered, with extra options", server_side_hello_world_with_options_path %>
  • @@ -59,9 +65,6 @@
  • <%= link_to "XHR Refresh", xhr_refresh_path %>
  • -
  • - <%= link_to "One Page with Many Examples at Once", root_path %> -
  • <%= link_to "Manually Rendered Component", client_side_manual_render_path %>
  • diff --git a/spec/dummy/client/app/components/HelloWorldHooks.jsx b/spec/dummy/client/app/components/HelloWorldHooks.jsx new file mode 100644 index 000000000..a27001ace --- /dev/null +++ b/spec/dummy/client/app/components/HelloWorldHooks.jsx @@ -0,0 +1,19 @@ +// Super simple example of the simplest possible React component +import React, { useState } from 'react'; +import css from './HelloWorld.scss'; + +// TODO: make more like the HelloWorld.jsx +function HelloWorldHooks(props) { + const [name, setName] = useState(props.helloWorldData.name); + return ( +
    +

    Hello, {name}!

    +

    + Say hello to: + setName(e.target.value)} /> +

    +
    + ); +} + +export default HelloWorldHooks; diff --git a/spec/dummy/client/app/components/HelloWorldHooksContext.jsx b/spec/dummy/client/app/components/HelloWorldHooksContext.jsx new file mode 100644 index 000000000..3a2e2f989 --- /dev/null +++ b/spec/dummy/client/app/components/HelloWorldHooksContext.jsx @@ -0,0 +1,32 @@ +// Example of using hooks when taking the props and railsContext +// Note, you need the call the hooks API within the react component stateless function +import React, { useState } from 'react'; +import css from './HelloWorld.scss'; +import RailsContext from './RailsContext'; + +/** + * NOTE: This exampe + * @param props + * @param railsContext + * @returns {function(): *} + * @constructor + */ +const HelloWorldHooksContext = (props, railsContext) => { + // You could pass props here or use the closure + return () => { + const [name, setName] = useState(props.helloWorldData.name); + return ( + <> +

    Hello, {name}!

    +

    + Say hello to: + setName(e.target.value)} /> +

    +

    Rails Context :

    + + + ); + }; +}; + +export default HelloWorldHooksContext; diff --git a/spec/dummy/client/app/packs/clientRegistration.jsx b/spec/dummy/client/app/packs/clientRegistration.jsx index c3e5a2e59..89a5cfe20 100644 --- a/spec/dummy/client/app/packs/clientRegistration.jsx +++ b/spec/dummy/client/app/packs/clientRegistration.jsx @@ -4,6 +4,8 @@ import 'es5-shim'; import ReactOnRails from 'react-on-rails'; import HelloWorld from '../components/HelloWorld'; +import HelloWorldHooks from '../components/HelloWorldHooks'; +import HelloWorldHooksContext from '../components/HelloWorldHooksContext'; import HelloWorldWithLogAndThrow from '../components/HelloWorldWithLogAndThrow'; import HelloWorldES5 from '../components/HelloWorldES5'; @@ -55,6 +57,8 @@ ReactOnRails.register({ ReactHelmetApp, ImageExample, SetTimeoutLoggingApp, + HelloWorldHooks, + HelloWorldHooksContext, }); ReactOnRails.registerStore({ diff --git a/spec/dummy/client/app/startup/ClientRenderedHtml.jsx b/spec/dummy/client/app/startup/ClientRenderedHtml.jsx index 86fc4dce7..90269312f 100644 --- a/spec/dummy/client/app/startup/ClientRenderedHtml.jsx +++ b/spec/dummy/client/app/startup/ClientRenderedHtml.jsx @@ -12,4 +12,11 @@ import EchoProps from '../components/EchoProps'; * Note, this is a fictional example, as you'd only use a generator function if you wanted to run * some extra code, such as setting up Redux and React-Router. */ -export default (props, _railsContext) => ; + +// This will not work: +// export default (props, _railsContext) => () => ; + +// You may do either: +// export default (props, _railsContext) => () => ; +// or +export default (props) => ; diff --git a/spec/dummy/client/app/startup/serverRegistration.jsx b/spec/dummy/client/app/startup/serverRegistration.jsx index 11a653eb8..ff3d88a06 100644 --- a/spec/dummy/client/app/startup/serverRegistration.jsx +++ b/spec/dummy/client/app/startup/serverRegistration.jsx @@ -6,6 +6,8 @@ import HelloString from '../non_react/HelloString'; // React components import HelloWorld from '../components/HelloWorld'; +import HelloWorldHooks from '../components/HelloWorldHooks'; +import HelloWorldHooksContext from '../components/HelloWorldHooksContext'; import HelloWorldES5 from '../components/HelloWorldES5'; import HelloWorldRehydratable from '../components/HelloWorldRehydratable'; @@ -66,6 +68,8 @@ ReactOnRails.register({ ReactHelmetApp, ImageExample, SetTimeoutLoggingApp, + HelloWorldHooks, + HelloWorldHooksContext, }); ReactOnRails.registerStore({ diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 8b0846907..a47efd7e5 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -15,6 +15,7 @@ get "server_side_hello_world_shared_store_controller" => "pages#server_side_hello_world_shared_store_controller" get "server_side_hello_world_shared_store_defer" => "pages#server_side_hello_world_shared_store_defer" get "server_side_hello_world" => "pages#server_side_hello_world" + get "server_side_hello_world_hooks" => "pages#server_side_hello_world_hooks" get "client_side_log_throw" => "pages#client_side_log_throw" get "server_side_log_throw" => "pages#server_side_log_throw" get "server_side_log_throw_plain_js" => "pages#server_side_log_throw_plain_js" diff --git a/spec/dummy/spec/system/integration_spec.rb b/spec/dummy/spec/system/integration_spec.rb index 8b2f3e517..5642ba57a 100644 --- a/spec/dummy/spec/system/integration_spec.rb +++ b/spec/dummy/spec/system/integration_spec.rb @@ -63,6 +63,15 @@ def finished_all_ajax_requests? context "Non-React Component" do scenario { is_expected.to have_content "Time to visit Maui" } end + + context "React Hooks" do + context "Simple stateless component" do + include_examples "React Component", "div#HelloWorldApp-react-component-6" + end + context "Generator function that takes props" do + include_examples "React Component", "div#HelloWorldApp-react-component-7" + end + end end context "Server Rendering with Options" do diff --git a/spec/dummy/yarn.lock b/spec/dummy/yarn.lock index 54f343adc..decdace16 100644 --- a/spec/dummy/yarn.lock +++ b/spec/dummy/yarn.lock @@ -8310,14 +8310,14 @@ raw-body@2.4.0: unpipe "1.0.0" react-dom@^16.8.6: - version "16.12.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.12.0.tgz#0da4b714b8d13c2038c9396b54a92baea633fe11" - integrity sha512-LMxFfAGrcS3kETtQaCkTKjMiifahaMySFDn71fZUNpPHZQEzmk/GiAeIT8JSOrHB23fnuCOMruL2a8NYlw+8Gw== + version "16.13.1" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.13.1.tgz#c1bd37331a0486c078ee54c4740720993b2e0e7f" + integrity sha512-81PIMmVLnCNLO/fFOQxdQkvEq/+Hfpv24XNJfpyZhTRfO0QcmQIF/PgCa1zCOj2w1hrn12MFLyaJ/G0+Mxtfag== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "^0.18.0" + scheduler "^0.19.1" react-fast-compare@^2.0.4: version "2.0.4" @@ -8355,11 +8355,16 @@ react-input-autosize@^2.2.1: dependencies: prop-types "^15.5.8" -react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.6: +react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.6: version "16.12.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.12.0.tgz#2cc0fe0fba742d97fd527c42a13bec4eeb06241c" integrity sha512-rPCkf/mWBtKc97aLL9/txD8DZdemK0vkA3JMLShjlJB3Pj3s+lpf1KaBzMfQrAmhMQB0n1cU/SUGgKKBCe837Q== +react-is@^16.8.1: + version "16.13.1" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" + integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== + react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" @@ -8434,9 +8439,9 @@ react-transition-group@^2.2.1: react-lifecycles-compat "^3.0.4" react@^16.8.6: - version "16.12.0" - resolved "https://registry.yarnpkg.com/react/-/react-16.12.0.tgz#0c0a9c6a142429e3614834d5a778e18aa78a0b83" - integrity sha512-fglqy3k5E+81pA8s+7K0/T3DBCF0ZDOher1elBFzF7O6arXJgzyu/FW+COxFvAWXJoJN9KIZbT2LXlukwphYTA== + version "16.13.1" + resolved "https://registry.yarnpkg.com/react/-/react-16.13.1.tgz#2e818822f1a9743122c063d6410d85c1e3afe48e" + integrity sha512-YMZQQq32xHLX0bz5Mnibv1/LHb3Sqzngu7xstSM+vrkE5Kzr9xE0yMByK5kMoTK30YVJE61WfbxIFFvfeDKT1w== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" @@ -9013,10 +9018,10 @@ saxes@^3.1.3: dependencies: xmlchars "^2.1.1" -scheduler@^0.18.0: - version "0.18.0" - resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.18.0.tgz#5901ad6659bc1d8f3fdaf36eb7a67b0d6746b1c4" - integrity sha512-agTSHR1Nbfi6ulI0kYNK0203joW2Y5W4po4l+v03tOoiJKpTBbxpNhWDvqc/4IcOw+KLmSiQLTasZ4cab2/UWQ== +scheduler@^0.19.1: + version "0.19.1" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.19.1.tgz#4f3e2ed2c1a7d65681f4c854fa8c5a1ccb40f196" + integrity sha512-n/zwRWRYSUj0/3g/otKDRPMh6qv2SYMWNq85IEa8iZyAv8od9zDYpGSnpBEjNgcMNq6Scbu5KfIPxNF72R/2EA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" diff --git a/yarn.lock b/yarn.lock index d17749d14..9442119f0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7769,14 +7769,14 @@ react-deep-force-update@^1.0.0: integrity sha512-WUSQJ4P/wWcusaH+zZmbECOk7H5N2pOIl0vzheeornkIMhu+qrNdGFm0bDZLCb0hSF0jf/kH1SgkNGfBdTc4wA== react-dom@^16.5.2: - version "16.6.3" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.6.3.tgz#8fa7ba6883c85211b8da2d0efeffc9d3825cccc0" - integrity sha512-8ugJWRCWLGXy+7PmNh8WJz3g1TaTUt1XyoIcFN+x0Zbkoz+KKdUyx1AQLYJdbFXjuF41Nmjn5+j//rxvhFjgSQ== + version "16.13.1" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.13.1.tgz#c1bd37331a0486c078ee54c4740720993b2e0e7f" + integrity sha512-81PIMmVLnCNLO/fFOQxdQkvEq/+Hfpv24XNJfpyZhTRfO0QcmQIF/PgCa1zCOj2w1hrn12MFLyaJ/G0+Mxtfag== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "^0.11.2" + scheduler "^0.19.1" react-is@^16.12.0: version "16.13.0" @@ -7805,14 +7805,13 @@ react-transform-hmr@^1.0.4: react-proxy "^1.1.7" react@^16.5.2: - version "16.6.3" - resolved "https://registry.yarnpkg.com/react/-/react-16.6.3.tgz#25d77c91911d6bbdd23db41e70fb094cc1e0871c" - integrity sha512-zCvmH2vbEolgKxtqXL2wmGCUxUyNheYn/C+PD1YAjfxHC54+MhdruyhO7QieQrYsYeTxrn93PM2y0jRH1zEExw== + version "16.13.1" + resolved "https://registry.yarnpkg.com/react/-/react-16.13.1.tgz#2e818822f1a9743122c063d6410d85c1e3afe48e" + integrity sha512-YMZQQq32xHLX0bz5Mnibv1/LHb3Sqzngu7xstSM+vrkE5Kzr9xE0yMByK5kMoTK30YVJE61WfbxIFFvfeDKT1w== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "^0.11.2" read-pkg-up@^1.0.1: version "1.0.1" @@ -8414,10 +8413,10 @@ saxes@^3.1.9: dependencies: xmlchars "^2.1.1" -scheduler@^0.11.2: - version "0.11.3" - resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.11.3.tgz#b5769b90cf8b1464f3f3cfcafe8e3cd7555a2d6b" - integrity sha512-i9X9VRRVZDd3xZw10NY5Z2cVMbdYg6gqFecfj79USv1CFN+YrJ3gIPRKf1qlY+Sxly4djoKdfx1T+m9dnRB8kQ== +scheduler@^0.19.1: + version "0.19.1" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.19.1.tgz#4f3e2ed2c1a7d65681f4c854fa8c5a1ccb40f196" + integrity sha512-n/zwRWRYSUj0/3g/otKDRPMh6qv2SYMWNq85IEa8iZyAv8od9zDYpGSnpBEjNgcMNq6Scbu5KfIPxNF72R/2EA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" From 90c3777588c0c60d9b81f6a04a4b7454d428567d Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 15 Apr 2020 19:25:51 -1000 Subject: [PATCH 02/10] docs --- CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7358c302..519e40fe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,19 +31,21 @@ Registered components may either do one of the following if functions: ##### Correct Either of these will work: -1. Take 2 props and return a function that takes zero or one params and returns JSX. +1. Take **2 params** and return **a function** that takes zero or one params and returns a + JSX or String. ```js - export default (props, _railsContext) => () => ; + export default (props, _railsContext) => () => ; ``` -2. Take only zero or one params and you return a JSX literal +2. Take only zero or one params and you return a JSX literal or String ```js - export default (props) => ; + export default (props) => ; ``` ##### Broken, as this function takes two params and it returns a JSX Literal ```js -export default (props, _railsContext) => ; +export default (props, _railsContext) => ; ``` +In this example, you need to wrap the `` in a function call. ### [11.3.0] - 2019-05-24 #### Added From 7da2ecf9f0a56e21e5fe823cd15ea96d21da9984 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 15 Apr 2020 20:22:20 -1000 Subject: [PATCH 03/10] Refinements --- CHANGELOG.md | 34 +++++--------- docs/basics/upgrading-react-on-rails.md | 44 +++++++++++++++++++ .../context_function_return_jsx.html.erb | 6 +++ ...ure_component_wrapped_in_function.html.erb | 15 +++++++ spec/dummy/app/views/shared/_header.erb | 6 +++ .../components/ContextFunctionReturnJSX.jsx | 25 +++++++++++ .../app/components/HelloWorldHooksContext.jsx | 7 --- .../PureComponentWrappedInFunction.jsx | 5 +++ .../client/app/packs/clientRegistration.jsx | 4 ++ spec/dummy/config/routes.rb | 2 + 10 files changed, 117 insertions(+), 31 deletions(-) create mode 100644 spec/dummy/app/views/pages/context_function_return_jsx.html.erb create mode 100644 spec/dummy/app/views/pages/pure_component_wrapped_in_function.html.erb create mode 100644 spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx create mode 100644 spec/dummy/client/app/components/PureComponentWrappedInFunction.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 519e40fe1..834951713 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,16 @@ Changes since last non-beta release. *Please add entries here for your pull requests that are not yet released.* +### [12.0.0] +#### BREAKING CHANGE +In order to solve the issues regarding React Hooks compatability, the number of parameters +for functions is used to determine if you have a generator function that will get invoked to +return a React component, or you are registering a functional React component. +[PR 1268](https://github.com/shakacode/react_on_rails/pull/1268) by [justin808](https://github.com/justin808) + +See [docs/basics/upgrading-react-on-rails](./docs/basics/upgrading-react-on-rails#upgrading-to-v12) +for details. + * Requires the use of rails/webpacker helpers * Removed **env_javascript_include_tag** and **env_stylesheet_link_tag** as these are replaced by view helpers from rails/webpacker @@ -23,30 +33,6 @@ Changes since last non-beta release. * Removal of config.symlink_non_digested_assets_regex as it's no longer needed with rails/webpacker. If any business needs this, we can move the code to a separate gem. -### [12.0.0] -#### BREAKING CHANGE -In order to solve the issues regarding React Hooks compatability: - -Registered components may either do one of the following if functions: - -##### Correct -Either of these will work: -1. Take **2 params** and return **a function** that takes zero or one params and returns a - JSX or String. - ```js - export default (props, _railsContext) => () => ; - ``` - -2. Take only zero or one params and you return a JSX literal or String - ```js - export default (props) => ; - ``` -##### Broken, as this function takes two params and it returns a JSX Literal -```js -export default (props, _railsContext) => ; -``` -In this example, you need to wrap the `` in a function call. - ### [11.3.0] - 2019-05-24 #### Added - Added method for retrieving any option from `render_options` [PR 1213](https://github.com/shakacode/react_on_rails/pull/1213) diff --git a/docs/basics/upgrading-react-on-rails.md b/docs/basics/upgrading-react-on-rails.md index 7850214a7..382d19588 100644 --- a/docs/basics/upgrading-react-on-rails.md +++ b/docs/basics/upgrading-react-on-rails.md @@ -5,6 +5,50 @@ If you would like help in migrating between React on Rails versions or help with We specialize in helping companies to quickly and efficiently move from versions before 9 to current. The older versions use the Rails asset pipeline to package client assets. The current and recommended way is to use Webpack 4 for asset preparation. You may also need help migrating from the `rails/webpacker`'s Webpack configuration to a better setup ready for Server Side Rendering. +## Upgrading to v12 +* Make sure that are are on a relatively more recent version of rails and webpacker. +* Updated API for ReactOnRails.register. + +In order to solve the issues regarding React Hooks compatability, the number of parameters +for functions is used to determine if you have a generator function that will get invoked to +return a React component, or you are registering a React component defined by a function. + +Registered component functions may either do one of the following: + +##### Correct +Either of these will work: +1. Take **2 params** and return **a function** that takes zero or one params and returns a + JSX or String. + ```js + export default (props, _railsContext) => () => ; + ``` + +2. Take only zero or one params and you return a JSX literal or String + ```js + export default (props) => ; + ``` +##### Broken, as this function takes two params and it returns a JSX Literal +```js +export default (props, _railsContext) => ; +``` +In this example, you need to wrap the `` in a function call, like this: + +```js +export default (props, _railsContext) => () => ; +``` + +If you make this mistake, you'll get this warning +`Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: . Did you accidentally export a JSX literal instead of a component?` + +And this error: +`react-dom.development.js:23965 Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.` + +If you have a pure component, taking one or zero parameters, and you have an unnecessary function +wrapper, then: + +1. You won't see anything render. +2. You will see this warning in development mode: `Warning: Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it.` + ## Upgrading rails/webpacker from v3 to v4 ### Custom Webpack build file The default value for `extract_css` is **false** in `config/webpack.yml`. Custom webpack builds should set this value to true or else no CSS link tags are generated. You have a custom webpack build if you are not using [rails/webpacker](https://github.com/rails/webpacker to setup your Webpack configuration. diff --git a/spec/dummy/app/views/pages/context_function_return_jsx.html.erb b/spec/dummy/app/views/pages/context_function_return_jsx.html.erb new file mode 100644 index 000000000..b4d71aacc --- /dev/null +++ b/spec/dummy/app/views/pages/context_function_return_jsx.html.erb @@ -0,0 +1,6 @@ +<%= react_component("ContextFunctionReturnJSX", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %> +
    + +

    Example of error from failing to wrap the result in a function.

    + +See console log for expected warning and errors. diff --git a/spec/dummy/app/views/pages/pure_component_wrapped_in_function.html.erb b/spec/dummy/app/views/pages/pure_component_wrapped_in_function.html.erb new file mode 100644 index 000000000..2e2a1af0d --- /dev/null +++ b/spec/dummy/app/views/pages/pure_component_wrapped_in_function.html.erb @@ -0,0 +1,15 @@ +<%= react_component("PureComponentWrappedInFunction", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %> + +
    + +

    Example of placing a pure component function in an unnecessary wrapper.

    + +

    + You will not see the component render and you will see this warning: +

    + +
    +  Warning: Functions are not valid as a React child. This may happen if you return
    +  a Component instead of  from render. Or maybe you meant to call this
    +  function rather than return it.
    +
    diff --git a/spec/dummy/app/views/shared/_header.erb b/spec/dummy/app/views/shared/_header.erb index 407f7837d..f2d575298 100644 --- a/spec/dummy/app/views/shared/_header.erb +++ b/spec/dummy/app/views/shared/_header.erb @@ -95,5 +95,11 @@
  • <%= link_to "Server Render With Timout", server_render_with_timeout_path %>
  • +
  • + <%= link_to "Incorrectly returning JSX rather than function", context_function_return_jsx_path %> +
  • +
  • + <%= link_to "Incorrectly wrapping a pure component in a function", pure_component_wrapped_in_function_path %> +

diff --git a/spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx b/spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx new file mode 100644 index 000000000..0a5962fa5 --- /dev/null +++ b/spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx @@ -0,0 +1,25 @@ +// Example of incorrectly taking two params and returning JSX +import React, { useState } from 'react'; +import css from './HelloWorld.scss'; +import RailsContext from './RailsContext'; + +const ContextFunctionReturnJSX = (props, railsContext) => ( + <> +

Hello, {props.name}!

+

Rails Context :

+ + +); + +/* Wrapping in a function would be correct in this case, since two params + are passed to the registered function: +const ContextFunctionReturnJSX = (props, railsContext) => () => ( + <> +

Hello, {props.name}!

+

Rails Context :

+ + +); + */ + +export default ContextFunctionReturnJSX; diff --git a/spec/dummy/client/app/components/HelloWorldHooksContext.jsx b/spec/dummy/client/app/components/HelloWorldHooksContext.jsx index 3a2e2f989..12eecf5a5 100644 --- a/spec/dummy/client/app/components/HelloWorldHooksContext.jsx +++ b/spec/dummy/client/app/components/HelloWorldHooksContext.jsx @@ -4,13 +4,6 @@ import React, { useState } from 'react'; import css from './HelloWorld.scss'; import RailsContext from './RailsContext'; -/** - * NOTE: This exampe - * @param props - * @param railsContext - * @returns {function(): *} - * @constructor - */ const HelloWorldHooksContext = (props, railsContext) => { // You could pass props here or use the closure return () => { diff --git a/spec/dummy/client/app/components/PureComponentWrappedInFunction.jsx b/spec/dummy/client/app/components/PureComponentWrappedInFunction.jsx new file mode 100644 index 000000000..ee98c7f61 --- /dev/null +++ b/spec/dummy/client/app/components/PureComponentWrappedInFunction.jsx @@ -0,0 +1,5 @@ +/* eslint-disable react/prop-types */ +import React from 'react'; + +// The extra function wrapper is unnecessary here +export default (props) => () =>

{props.title}

; diff --git a/spec/dummy/client/app/packs/clientRegistration.jsx b/spec/dummy/client/app/packs/clientRegistration.jsx index 89a5cfe20..797051e54 100644 --- a/spec/dummy/client/app/packs/clientRegistration.jsx +++ b/spec/dummy/client/app/packs/clientRegistration.jsx @@ -6,6 +6,8 @@ import ReactOnRails from 'react-on-rails'; import HelloWorld from '../components/HelloWorld'; import HelloWorldHooks from '../components/HelloWorldHooks'; import HelloWorldHooksContext from '../components/HelloWorldHooksContext'; +import ContextFunctionReturnJSX from '../components/ContextFunctionReturnJSX'; +import PureComponentWrappedInFunction from '../components/PureComponentWrappedInFunction'; import HelloWorldWithLogAndThrow from '../components/HelloWorldWithLogAndThrow'; import HelloWorldES5 from '../components/HelloWorldES5'; @@ -59,6 +61,8 @@ ReactOnRails.register({ SetTimeoutLoggingApp, HelloWorldHooks, HelloWorldHooksContext, + ContextFunctionReturnJSX, + PureComponentWrappedInFunction, }); ReactOnRails.registerStore({ diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index a47efd7e5..6865372b5 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -39,4 +39,6 @@ get "broken_app" => "pages#broken_app" get "image_example" => "pages#image_example" get "server_render_with_timeout" => "pages#server_render_with_timeout" + get "context_function_return_jsx" => "pages#context_function_return_jsx" + get "pure_component_wrapped_in_function" => "pages#pure_component_wrapped_in_function" end From 0ad1ef285c180cdd530d8a5315520d3ba5cdb6b1 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Thu, 16 Apr 2020 16:45:57 -1000 Subject: [PATCH 04/10] Before renaming generatorFunction to reactComponent After this commit, going to change the logic to reverse the check for reactComponent vs. Generator function --- CHANGELOG.md | 4 +- README.md | 21 ++++++++- docs/additional-reading/react-helmet.md | 1 + lib/react_on_rails/helper.rb | 46 ++++++++++++------- node_package/src/ComponentRegistry.ts | 6 +-- node_package/src/ReactOnRails.ts | 8 ++-- node_package/src/clientStartup.ts | 13 +++--- ...teReactElement.ts => createReactOutput.ts} | 25 +++++----- node_package/src/generatorFunction.ts | 20 +++++--- ...eateReactElementResultNonReactComponent.ts | 9 ---- node_package/src/isServerRenderResult.ts | 11 +++++ .../src/serverRenderReactComponent.ts | 6 +-- node_package/src/types/index.d.ts | 37 ++++++++++----- node_package/tests/ComponentRegistry.test.js | 28 +++++++++-- node_package/tests/generatorFunction.test.js | 7 +-- package.json | 6 +-- .../app/controllers/application_controller.rb | 2 +- .../app/views/pages/react_helmet_broken.erb | 22 +++++++++ spec/dummy/app/views/shared/_header.erb | 3 ++ .../app/startup/ReactHelmetClientApp.jsx | 8 ++++ .../app/startup/ReactHelmetServerApp.jsx | 5 ++ .../startup/ReactHelmetServerAppBroken.jsx | 29 ++++++++++++ .../client/app/startup/serverRegistration.jsx | 8 +++- spec/dummy/config/initializers/listen.rb | 14 ------ spec/dummy/config/routes.rb | 1 + spec/dummy/package.json | 2 +- spec/dummy/spec/system/integration_spec.rb | 12 +++++ webpackConfigLoader.js | 2 + 28 files changed, 250 insertions(+), 106 deletions(-) rename node_package/src/{createReactElement.ts => createReactOutput.ts} (62%) delete mode 100644 node_package/src/isCreateReactElementResultNonReactComponent.ts create mode 100644 node_package/src/isServerRenderResult.ts create mode 100644 spec/dummy/app/views/pages/react_helmet_broken.erb create mode 100644 spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx delete mode 100644 spec/dummy/config/initializers/listen.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 834951713..6cc5122d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,9 @@ Changes since last non-beta release. #### BREAKING CHANGE In order to solve the issues regarding React Hooks compatability, the number of parameters for functions is used to determine if you have a generator function that will get invoked to -return a React component, or you are registering a functional React component. +return a React component, or you are registering a functional React component. Alternately, you can +set JavaScript property `generatorFunction` on the function for which you want to return to be +invoked to return the React component. In that case, you won't need to pass any unused params. [PR 1268](https://github.com/shakacode/react_on_rails/pull/1268) by [justin808](https://github.com/justin808) See [docs/basics/upgrading-react-on-rails](./docs/basics/upgrading-react-on-rails#upgrading-to-v12) diff --git a/README.md b/README.md index 817385e9d..ad38f7b74 100644 --- a/README.md +++ b/README.md @@ -229,13 +229,30 @@ For details on techniques to use different code for client and server rendering, ## Specifying Your React Components: Direct or Generator Functions -You have two ways to specify your React components. You can either register the React component directly, or you can create a function that returns a React component. 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 "generator function". Creating a function has the following benefits: 1. You have access to the `railsContext`. See documentation for the railsContext in terms of why you might need it. You **need** a generator 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. -ReactOnRails will automatically detect a registered generator function. Thus, there is no difference between registering a React Component versus a "generator function." +Note, the return value of a **generator function** should be JSX or an HTML string. Do not return a +function. + +ReactOnRails will automatically detect a registered generator 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 +React component, then you need to specify at least a second parameter. This is the `railsContext`. +If you're not using this parameter, declare your function with the unused param: + +```js +const MyComponentGenerator = (props, _railsContext) => { + + if (props.print) { + return

{JSON.stringify(props)}

; + } + +``` + +Thus, there is no difference between registering a React function or class Component versus a "generator function." ## react_component_hash for Generator Functions diff --git a/docs/additional-reading/react-helmet.md b/docs/additional-reading/react-helmet.md index 989949981..72bb24032 100644 --- a/docs/additional-reading/react-helmet.md +++ b/docs/additional-reading/react-helmet.md @@ -55,6 +55,7 @@ ReactOnRails.register({ }); ``` ```javascript +// Note the import from the server file. import ReactHelmetApp from '../ReactHelmetServerApp'; ReactOnRails.register({ diff --git a/lib/react_on_rails/helper.rb b/lib/react_on_rails/helper.rb index 60ec4cead..e1515911f 100644 --- a/lib/react_on_rails/helper.rb +++ b/lib/react_on_rails/helper.rb @@ -17,13 +17,18 @@ module Helper COMPONENT_HTML_KEY = "componentHtml" - # react_component_name: can be a React component, created using a ES6 class, or - # React.createClass, or a - # `generator function` that returns a React component - # using ES6 - # let MyReactComponentApp = (props, railsContext) => ; - # or using ES5 - # var MyReactComponentApp = function(props, railsContext) { return ; } + # react_component_name: can be a React function or class component or a "generator function". + # "generator functions" differ from a React function in that they take two parameters, the + # props and the railsContext, like this: + # + # let MyReactComponentApp = (props, railsContext) => ; + # + # Alternately, you can define the generator function with an additional property + # `.generatorFunction = true`: + # + # let MyReactComponentApp = (props) => ; + # MyReactComponent.generatorFunction = true; + # # Exposing the react_component_name is necessary to both a plain ReactComponent as well as # a generator: # See README.md for how to "register" your react components. @@ -45,7 +50,8 @@ module Helper # raise_on_prerender_error: Default to false. True will raise exception on server # if the JS code throws # Any other options are passed to the content tag, including the id. - # random_dom_id can be set to override the global default. + # random_dom_id can be set to override the default from the config/initializers. That's only + # used if you have multiple instance of the same component on the Rails view. def react_component(component_name, options = {}) internal_result = internal_react_component(component_name, options) server_rendered_html = internal_result[:result]["html"] @@ -59,20 +65,24 @@ def react_component(component_name, options = {}) render_options: internal_result[:render_options] ) elsif server_rendered_html.is_a?(Hash) - msg = <<-MSG.strip_heredoc + msg = <<~MSG Use react_component_hash (not react_component) to return a Hash to your ruby view code. See https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx - for an example of the necessary javascript configuration." + for an example of the necessary javascript configuration. MSG raise ReactOnRails::Error, msg - else - msg = <<-MSG.strip_heredoc - ReactOnRails: server_rendered_html is expected to be a String for #{component_name}. If you're - trying to use a generator function to return a Hash to your ruby view code, then use + class_name = server_rendered_html.class.name + msg = <<~MSG + ReactOnRails: server_rendered_html is expected to be a String or Hash for #{component_name}. + Type is #{class_name} + Value: + #{server_rendered_html} + + If you're trying to use a generator function to return a Hash to your ruby view code, then use react_component_hash instead of react_component and see https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx - for an example of the JavaScript code." + for an example of the JavaScript code. MSG raise ReactOnRails::Error, msg end @@ -113,10 +123,12 @@ def react_component_hash(component_name, options = {}) render_options: internal_result[:render_options] ) else - msg = <<-MSG.strip_heredoc + msg = <<~MSG Generator function used by react_component_hash for #{component_name} is expected to return an Object. See https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx - for an example of the JavaScript code." + for an example of the JavaScript code. + Note, your generator function must either take 2 params or have the property + `.generatorFunction = true` added to it to distinguish it from a React Function Component. MSG raise ReactOnRails::Error, msg end diff --git a/node_package/src/ComponentRegistry.ts b/node_package/src/ComponentRegistry.ts index 0b00137e6..231417ceb 100644 --- a/node_package/src/ComponentRegistry.ts +++ b/node_package/src/ComponentRegistry.ts @@ -1,4 +1,4 @@ -import type { RegisteredComponent, ComponentOrRenderFunction, RenderFunction } from './types/index'; +import type { RegisteredComponent, ComponentOrGeneratorFunction, GeneratorFunction } from './types/index'; import generatorFunction from './generatorFunction'; const registeredComponents = new Map(); @@ -7,7 +7,7 @@ export default { /** * @param components { component1: component1, component2: component2, etc. } */ - register(components: { [id: string]: ComponentOrRenderFunction }): void { + register(components: { [id: string]: ComponentOrGeneratorFunction }): void { Object.keys(components).forEach(name => { if (registeredComponents.has(name)) { console.warn('Called register for component that is already registered', name); @@ -19,7 +19,7 @@ export default { } const isGeneratorFunction = generatorFunction(component); - const isRenderer = isGeneratorFunction && (component as RenderFunction).length === 3; + const isRenderer = isGeneratorFunction && (component as GeneratorFunction).length === 3; registeredComponents.set(name, { name, diff --git a/node_package/src/ReactOnRails.ts b/node_package/src/ReactOnRails.ts index fffbbd2ad..ee702dde2 100644 --- a/node_package/src/ReactOnRails.ts +++ b/node_package/src/ReactOnRails.ts @@ -8,14 +8,14 @@ import ComponentRegistry from './ComponentRegistry'; import StoreRegistry from './StoreRegistry'; import serverRenderReactComponent from './serverRenderReactComponent'; import buildConsoleReplay from './buildConsoleReplay'; -import createReactElement from './createReactElement'; +import createReactOutput from './createReactOutput'; import Authenticity from './Authenticity'; import context from './context'; import type { RegisteredComponent, RenderParams, ErrorOptions, - ComponentOrRenderFunction, + ComponentOrGeneratorFunction, AuthenticityHeaders, StoreGenerator } from './types/index'; @@ -37,7 +37,7 @@ ctx.ReactOnRails = { * find you components for rendering. * @param components (key is component name, value is component) */ - register(components: { [id: string]: ComponentOrRenderFunction }): void { + register(components: { [id: string]: ComponentOrGeneratorFunction }): void { ComponentRegistry.register(components); }, @@ -173,7 +173,7 @@ ctx.ReactOnRails = { */ render(name: string, props: Record, domNodeId: string, hydrate: boolean): void | Element | Component { const componentObj = ComponentRegistry.get(name); - const reactElement = createReactElement({ componentObj, props, domNodeId }); + const reactElement = createReactOutput({ componentObj, props, domNodeId }); const render = hydrate ? ReactDOM.hydrate : ReactDOM.render; // eslint-disable-next-line react/no-render-return-value diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index a7a34d718..1f62bfd15 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -4,11 +4,12 @@ import type { ReactOnRails as ReactOnRailsType, RailsContext, RegisteredComponent, - RenderFunction + GeneratorFunction, + GeneratorFunctionResult, } from './types/index'; -import createReactElement from './createReactElement'; -import isRouterResult from './isCreateReactElementResultNonReactComponent'; +import createReactOutput from './createReactOutput'; +import isServerRenderResult from './isServerRenderResult'; declare global { interface Window { @@ -113,7 +114,7 @@ DELEGATING TO RENDERER ${name} for dom node with id: ${domNodeId} with props, ra props, railsContext); } - (component as RenderFunction)(props, railsContext, domNodeId); + (component as GeneratorFunction)(props, railsContext, domNodeId); return true; } @@ -148,7 +149,7 @@ function render(el: Element, railsContext: RailsContext): void { // Hydrate if available and was server rendered const shouldHydrate = !!ReactDOM.hydrate && !!domNode.innerHTML; - const reactElementOrRouterResult = createReactElement({ + const reactElementOrRouterResult = createReactOutput({ componentObj, props, domNodeId, @@ -157,7 +158,7 @@ function render(el: Element, railsContext: RailsContext): void { shouldHydrate, }); - if (isRouterResult(reactElementOrRouterResult)) { + if (isServerRenderResult(reactElementOrRouterResult)) { throw new Error(`\ You returned a server side type of react-router error: ${JSON.stringify(reactElementOrRouterResult)} You should return a React.Component always for the client side entry point.`); diff --git a/node_package/src/createReactElement.ts b/node_package/src/createReactOutput.ts similarity index 62% rename from node_package/src/createReactElement.ts rename to node_package/src/createReactOutput.ts index 409845018..891b24a85 100644 --- a/node_package/src/createReactElement.ts +++ b/node_package/src/createReactOutput.ts @@ -1,8 +1,9 @@ /* eslint-disable react/prop-types */ import React from 'react'; -import type { CreateParams, ComponentVariant, RenderFunction, CREReturnTypes } from './types/index'; -import isRouterResult from "./isCreateReactElementResultNonReactComponent"; +import type { ServerRenderResult, + CreateParams, ReactComponentVariant, GeneratorFunction, CreateReactOutputResult } from './types/index'; +import isServerRenderResult from "./isServerRenderResult"; /** * Logic to either call the generatorFunction or call React.createElement to get the @@ -15,14 +16,14 @@ import isRouterResult from "./isCreateReactElementResultNonReactComponent"; * @param options.location * @returns {ReactElement} */ -export default function createReactElement({ +export default function createReactOutput({ componentObj, props, railsContext, domNodeId, trace, shouldHydrate, -}: CreateParams): CREReturnTypes { +}: CreateParams): CreateReactOutputResult { const { name, component, generatorFunction } = componentObj; if (trace) { @@ -37,19 +38,19 @@ export default function createReactElement({ } } - // TODO: replace any - let ReactComponent: any; if (generatorFunction) { // Let's invoke the function to get the result - ReactComponent = (component as RenderFunction)(props, railsContext); - if (isRouterResult(ReactComponent)) { + let result = (component as GeneratorFunction)(props, railsContext); + if (isServerRenderResult(result as CreateReactOutputResult)) { // 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 ReactComponent; - } // else we'll be calling React.createElement + return (result as ServerRenderResult); + } else { // else we'll be calling React.createElement + let reactComponent = result as ReactComponentVariant; + return React.createElement(reactComponent, props); + } } else { - ReactComponent = component; + return React.createElement(component as ReactComponentVariant, props); } - return React.createElement(ReactComponent as ComponentVariant, props); } diff --git a/node_package/src/generatorFunction.ts b/node_package/src/generatorFunction.ts index ed1619bea..124bb21ed 100644 --- a/node_package/src/generatorFunction.ts +++ b/node_package/src/generatorFunction.ts @@ -1,6 +1,6 @@ // See discussion: // https://discuss.reactjs.org/t/how-to-determine-if-js-object-is-react-component/2825/2 -import { ComponentOrRenderFunction, RenderFunction } from "./types/index"; +import { ComponentOrGeneratorFunction, GeneratorFunction } from "./types/index"; /** * Used to determine we'll call be calling React.createElement on the component of if this is a @@ -8,17 +8,23 @@ import { ComponentOrRenderFunction, RenderFunction } from "./types/index"; * @param component * @returns {boolean} */ -export default function generatorFunction(component: ComponentOrRenderFunction): boolean { - if (!component.prototype) { +export default function generatorFunction(component: ComponentOrGeneratorFunction): boolean { + // No for es5 or es6 React Component + if ( + (component as GeneratorFunction).prototype && + (component as GeneratorFunction).prototype.isReactComponent) { return false; } + if ((component as GeneratorFunction).generatorFunction) { + return true; + } + // If zero or one args, then we know that this is a regular function that will // return a React component - if (component.length < 2) { - return false; + if (component.length >= 2) { + return true; } - // es5 or es6 React Component - return !component.prototype.isReactComponent; + return false; } diff --git a/node_package/src/isCreateReactElementResultNonReactComponent.ts b/node_package/src/isCreateReactElementResultNonReactComponent.ts deleted file mode 100644 index 006f621fa..000000000 --- a/node_package/src/isCreateReactElementResultNonReactComponent.ts +++ /dev/null @@ -1,9 +0,0 @@ -import type { CREReturnTypes } from './types/index'; - -export default function isResultNonReactComponent(reactElementOrRouterResult: CREReturnTypes): boolean { - return !!( - (reactElementOrRouterResult as {renderedHtml: string}).renderedHtml || - (reactElementOrRouterResult as {redirectLocation: {pathname: string; search: string}}).redirectLocation || - (reactElementOrRouterResult as {routeError: Error}).routeError || - (reactElementOrRouterResult as {error: Error}).error); -} diff --git a/node_package/src/isServerRenderResult.ts b/node_package/src/isServerRenderResult.ts new file mode 100644 index 000000000..a217b5080 --- /dev/null +++ b/node_package/src/isServerRenderResult.ts @@ -0,0 +1,11 @@ +import type { CreateReactOutputResult, ServerRenderResult } from './types/index'; + +export default function isServerRenderResult(testValue: CreateReactOutputResult): + testValue is ServerRenderResult { + return !!( + (testValue as ServerRenderResult).renderedHtml || + (testValue as ServerRenderResult).redirectLocation || + (testValue as ServerRenderResult).routeError || + (testValue as ServerRenderResult).error); +} + diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 06f310b46..10fb87506 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -2,9 +2,9 @@ import ReactDOMServer from 'react-dom/server'; import type { ReactElement } from 'react'; import ComponentRegistry from './ComponentRegistry'; -import createReactElement from './createReactElement'; +import createReactOutput from './createReactOutput'; import isCreateReactElementResultNonReactComponent from - './isCreateReactElementResultNonReactComponent'; + './isServerRenderResult'; import buildConsoleReplay from './buildConsoleReplay'; import handleError from './handleError'; import type { RenderParams } from './types/index'; @@ -23,7 +23,7 @@ Detected a renderer while server rendering component '${name}'. \ See https://github.com/shakacode/react_on_rails#renderer-functions`); } - const reactElementOrRouterResult = createReactElement({ + const reactElementOrRouterResult = createReactOutput({ componentObj, domNodeId, trace, diff --git a/node_package/src/types/index.d.ts b/node_package/src/types/index.d.ts index 690e13ca8..fb2c7ad55 100644 --- a/node_package/src/types/index.d.ts +++ b/node_package/src/types/index.d.ts @@ -1,7 +1,8 @@ import type { ReactElement, Component, FunctionComponent, ComponentClass } from 'react'; import type { Store } from 'redux'; -type ComponentVariant = FunctionComponent | ComponentClass; + +type ReactComponentVariant = FunctionComponent | ComponentClass | string; interface Params { props?: {}; @@ -38,28 +39,42 @@ export interface RailsContext { httpAcceptLanguage?: string; } -type RenderFunction = (props?: {}, railsContext?: RailsContext, domNodeId?: string) => ReactElement; - -type ComponentOrRenderFunction = ComponentVariant | RenderFunction; +interface ServerRenderResult { + renderedHtml?: string; + redirectLocation?: {pathname: string; search: string}; + routeError?: Error; + error?: Error; +} type AuthenticityHeaders = {[id: string]: string} & {'X-CSRF-Token': string | null; 'X-Requested-With': string}; type StoreGenerator = (props: {}, railsContext: RailsContext) => Store -type CREReturnTypes = {renderedHtml: string} | {redirectLocation: {pathname: string; search: string}} | {routeError: Error} | {error: Error} | ReactElement | React.FC; +type CreateReactOutputResult = ServerRenderResult | ReactElement; + +type GeneratorFunctionResult = ReactComponentVariant | ServerRenderResult; + +interface GeneratorFunction { + (props?: {}, railsContext?: RailsContext, domNodeId?: string): GeneratorFunctionResult; + generatorFunction?: boolean; +} + +type ComponentOrGeneratorFunction = ReactComponentVariant | GeneratorFunction; export type { // eslint-disable-line import/prefer-default-export - ComponentOrRenderFunction, - ComponentVariant, + ComponentOrGeneratorFunction, + ReactComponentVariant, AuthenticityHeaders, - RenderFunction, + GeneratorFunction, + GeneratorFunctionResult, StoreGenerator, - CREReturnTypes + CreateReactOutputResult, + ServerRenderResult, } export interface RegisteredComponent { name: string; - component: ComponentOrRenderFunction; + component: ComponentOrGeneratorFunction; generatorFunction: boolean; isRenderer: boolean; } @@ -77,7 +92,7 @@ export interface ErrorOptions { } export interface ReactOnRails { - register(components: { [id: string]: ComponentOrRenderFunction }): void; + register(components: { [id: string]: ComponentOrGeneratorFunction }): void; registerStore(stores: { [id: string]: Store }): void; getStore(name: string, throwIfMissing: boolean): Store | undefined; setOptions(newOptions: {traceTurbolinks: boolean}): void; diff --git a/node_package/tests/ComponentRegistry.test.js b/node_package/tests/ComponentRegistry.test.js index 330245682..b7f04be27 100644 --- a/node_package/tests/ComponentRegistry.test.js +++ b/node_package/tests/ComponentRegistry.test.js @@ -12,11 +12,21 @@ import ComponentRegistry from '../src/ComponentRegistry'; describe('ComponentRegistry', () => { expect.assertions(9); - it('registers and retrieves generator function components', () => { + it('registers and retrieves React function components', () => { expect.assertions(1); const C1 = () =>
HELLO
; ComponentRegistry.register({ C1 }); const actual = ComponentRegistry.get('C1'); + const expected = { name: 'C1', component: C1, generatorFunction: false, isRenderer: false }; + expect(actual).toEqual(expected); + }); + + it('registers and retrieves generator function components where property generatorFunction is set and zero params', () => { + expect.assertions(1); + const C1 = () =>
HELLO
; + C1.generatorFunction = true; + ComponentRegistry.register({ C1 }); + const actual = ComponentRegistry.get('C1'); const expected = { name: 'C1', component: C1, generatorFunction: true, isRenderer: false }; expect(actual).toEqual(expected); }); @@ -47,7 +57,7 @@ describe('ComponentRegistry', () => { expect(actual).toEqual(expected); }); - it('registers and retrieves renderers', () => { + it('registers and retrieves renderers if 3 params', () => { expect.assertions(1); const C4 = (a1, a2, a3) => null; ComponentRegistry.register({ C4 }); @@ -62,21 +72,31 @@ describe('ComponentRegistry', () => { */ it('registers and retrieves multiple components', () => { expect.assertions(3); + // Plain react stateless functional components const C5 = () =>
WHY
; const C6 = () =>
NOW
; + const C7 = () =>
NOW
; + C7.generatorFunction = true; ComponentRegistry.register({ C5 }); ComponentRegistry.register({ C6 }); + ComponentRegistry.register({ C7 }); const components = ComponentRegistry.components(); expect(components.size).toBe(6); expect(components.get('C5')).toEqual({ name: 'C5', component: C5, - generatorFunction: true, + generatorFunction: false, isRenderer: false, }); expect(components.get('C6')).toEqual({ name: 'C6', component: C6, + generatorFunction: false, + isRenderer: false, + }); + expect(components.get('C7')).toEqual({ + name: 'C7', + component: C7, generatorFunction: true, isRenderer: false, }); @@ -98,7 +118,7 @@ describe('ComponentRegistry', () => { expect(components.get('C8')).toEqual({ name: 'C8', component: C8, - generatorFunction: true, + generatorFunction: false, isRenderer: false, }); }); diff --git a/node_package/tests/generatorFunction.test.js b/node_package/tests/generatorFunction.test.js index 4224362f0..e882ba0da 100644 --- a/node_package/tests/generatorFunction.test.js +++ b/node_package/tests/generatorFunction.test.js @@ -73,12 +73,7 @@ describe('generatorFunction', () => { it('returns true for a generator function (containing two params)', () => { expect.assertions(1); - const foobarComponent = createReactClass({ - render() { - return
Component for Generator Function
; - }, - }); - + const foobarComponent = () =>
Component for Generator Function
; const foobarGeneratorFunction = (_props, _railsContext) => foobarComponent; expect(generatorFunction(foobarGeneratorFunction)).toBe(true); diff --git a/package.json b/package.json index a7e05f854..f7fac9d11 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-on-rails", - "version": "11.3.0", + "version": "12.0.0", "description": "react-on-rails JavaScript for react_on_rails Ruby gem", "main": "node_package/lib/ReactOnRails.js", "directories": { @@ -54,8 +54,8 @@ }, "peerDependencies": { "js-yaml": ">= 3.0.0", - "react": ">= 0.14", - "react-dom": ">= 0.14" + "react": ">= 16", + "react-dom": ">= 16" }, "files": [ "node_package/lib", diff --git a/spec/dummy/app/controllers/application_controller.rb b/spec/dummy/app/controllers/application_controller.rb index 42be5c60d..e53df04ce 100644 --- a/spec/dummy/app/controllers/application_controller.rb +++ b/spec/dummy/app/controllers/application_controller.rb @@ -11,7 +11,7 @@ class ApplicationController < ActionController::Base Rails.logger.error("Caught ReactOnRails::PrerenderError in ApplicationController error handler.") Rails.logger.error(err.message) Rails.logger.error(err.backtrace.join("\n")) - msg = <<-MSG.strip_heredoc + msg = <<~MSG Error prerendering in react_on_rails. Redirected back to '/server_side_log_throw_raise_invoker'. See server logs for output. diff --git a/spec/dummy/app/views/pages/react_helmet_broken.erb b/spec/dummy/app/views/pages/react_helmet_broken.erb new file mode 100644 index 000000000..11aa267f3 --- /dev/null +++ b/spec/dummy/app/views/pages/react_helmet_broken.erb @@ -0,0 +1,22 @@ + +<% react_helmet_app = react_component_hash("ReactHelmetAppBroken", + props: { helloWorldData: { name: "Mr. Server Side Rendering"}}, + id: "react-helmet-0", + trace: true) %> + + +<% content_for :title do %> + <%= react_helmet_app['title'] %> +<% end %> + +<%= react_helmet_app["componentHtml"] %> + +
+ +This page demonstrates an improperly defined generator function. + +

Here's the error message when you forget to mark the function as a generator function:

+ +
+  Objects are not valid as a React child (found: object with keys {renderedHtml}). If you meant to render a collection of children, use an array instead.\n    in Unknown
+
diff --git a/spec/dummy/app/views/shared/_header.erb b/spec/dummy/app/views/shared/_header.erb index f2d575298..e5e5c2b2e 100644 --- a/spec/dummy/app/views/shared/_header.erb +++ b/spec/dummy/app/views/shared/_header.erb @@ -89,6 +89,9 @@
  • <%= link_to "Generator function returns object with renderedHtml as another object (react-helmet)", react_helmet_path %>
  • +
  • + <%= link_to "Improperly defined generator function (react-helmet)", react_helmet_broken_path %> +
  • <%= link_to "Image Example", image_example_path %>
  • diff --git a/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx b/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx index 4ffbd02ad..dbfdbecdc 100644 --- a/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx +++ b/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx @@ -2,4 +2,12 @@ import React from 'react'; import ReactHelmet from '../components/ReactHelmet'; +// This works fine, React functional component: +// export default (props) => ; + export default (props, _railsContext) => ; + +// Note, the server side has to be a generator function + +// If you want a generatorFunction, return a ReactComponent +// export default (props, _railsContext) => () => ; diff --git a/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx b/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx index 9dcf6bc34..aec7088a1 100644 --- a/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx +++ b/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx @@ -11,6 +11,9 @@ import ReactHelmet from '../components/ReactHelmet'; * * This is imported as "ReactHelmetApp" by "serverRegistration.jsx". Note that rendered * component markup must go under "componentHtml" key. + * + * Note that the function takes 2 params to identify this as a generator fuction. Alternately, + * the function could get the property of `.generatorFunction = true` added to it. */ export default (props, _railsContext) => { const componentHtml = renderToString(); @@ -20,5 +23,7 @@ export default (props, _railsContext) => { componentHtml, title: helmet.title.toString(), }; + + // Note that this function returns an Object for server rendering. return { renderedHtml }; }; diff --git a/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx b/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx new file mode 100644 index 000000000..39ae45d5e --- /dev/null +++ b/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx @@ -0,0 +1,29 @@ +// Top level component for simple client side only rendering +// This one is broken in that the function takes one param, so it's not a generator +// function. The point of this is to provide a good error. +import React from 'react'; +import { renderToString } from 'react-dom/server'; +import { Helmet } from 'react-helmet'; +import ReactHelmet from '../components/ReactHelmet'; + +/* + * Export a function that takes the props and returns an object with { renderedHtml } + * This example shows returning renderedHtml as an object itself that contains rendered + * component markup and additional HTML strings. + * + * This is imported as "ReactHelmetApp" by "serverRegistration.jsx". Note that rendered + * component markup must go under "componentHtml" key. + * + * Note that the function should take 2 params to identify this as a generator fuction. + * Alternately, the function could get the property of `.generatorFunction = true` added to it. + */ +export default (props) => { + const componentHtml = renderToString(); + const helmet = Helmet.renderStatic(); + + const renderedHtml = { + componentHtml, + title: helmet.title.toString(), + }; + return { renderedHtml }; +}; diff --git a/spec/dummy/client/app/startup/serverRegistration.jsx b/spec/dummy/client/app/startup/serverRegistration.jsx index ff3d88a06..c45a8fcf0 100644 --- a/spec/dummy/client/app/startup/serverRegistration.jsx +++ b/spec/dummy/client/app/startup/serverRegistration.jsx @@ -35,12 +35,15 @@ import SharedReduxStore from '../stores/SharedReduxStore'; import DeferredRenderApp from '../startup/DeferredRenderAppServer'; import ManualRenderApp from '../startup/ManualRenderAppRenderer'; -// Deferred render on the client side w/ server render import RenderedHtml from '../startup/ServerRenderedHtml'; -// Deferred render on the client side w/ server render with additional HTML strings: +// Server render with additional HTML strings: import ReactHelmetApp from '../startup/ReactHelmetServerApp'; +// Broken server render since ReactHelmetServerAppBroken is not properly defined +// to be a generatorFunction +import ReactHelmetAppBroken from '../startup/ReactHelmetServerAppBroken'; + // Demonstrate using Images import ImageExample from '../components/ImageExample'; @@ -66,6 +69,7 @@ ReactOnRails.register({ DeferredRenderApp, RenderedHtml, ReactHelmetApp, + ReactHelmetAppBroken, ImageExample, SetTimeoutLoggingApp, HelloWorldHooks, diff --git a/spec/dummy/config/initializers/listen.rb b/spec/dummy/config/initializers/listen.rb deleted file mode 100644 index 38dbfe4e6..000000000 --- a/spec/dummy/config/initializers/listen.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -# https://github.com/guard/listen/wiki/Duplicate-directory-errors -# Listen >=2.8 patch to silence duplicate directory errors. USE AT YOUR OWN RISK -require "listen/record/symlink_detector" -module Listen - class Record - class SymlinkDetector - def _fail(_xxx, _yyy) - raise Error, "Don't watch locally-symlinked directory twice" - end - end - end -end diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 6865372b5..554c04600 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -36,6 +36,7 @@ get "rendered_html" => "pages#rendered_html" get "xhr_refresh" => "pages#xhr_refresh" get "react_helmet" => "pages#react_helmet" + get "react_helmet_broken" => "pages#react_helmet_broken" get "broken_app" => "pages#broken_app" get "image_example" => "pages#image_example" get "server_render_with_timeout" => "pages#server_render_with_timeout" diff --git a/spec/dummy/package.json b/spec/dummy/package.json index 28080114d..a9e7f9cdf 100644 --- a/spec/dummy/package.json +++ b/spec/dummy/package.json @@ -44,7 +44,7 @@ "react-dom": "^16.8.6", "react-helmet": "^6.0.0-beta.2", "react-hot-loader": "^4.12.12", - "react-on-rails": "11.2.2", + "react-on-rails": "13.0.0", "react-proptypes": "^1.0.0", "react-redux": "^5.0.5", "react-router": "^3.2.1", diff --git a/spec/dummy/spec/system/integration_spec.rb b/spec/dummy/spec/system/integration_spec.rb index 5642ba57a..b11b26b9c 100644 --- a/spec/dummy/spec/system/integration_spec.rb +++ b/spec/dummy/spec/system/integration_spec.rb @@ -246,6 +246,18 @@ def finished_all_ajax_requests? end end + shared_examples "renderedHtmls should have errors" do + subject { page } + background { visit react_helmet_broken_path } + scenario "renderedHtmls should have errors" do + puts "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ" + puts "integration_spec.rb: #{__LINE__}, method: #{__method__}" + puts "subject.html = #{subject.html.ai}" + puts "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ" + expect(subject.html).to include("[SERVER] RENDERED ReactHelmetApp to dom node with id") + end + end + describe "with disabled JS" do include_examples "renderedHtmls should not have any errors and set correct page title" end diff --git a/webpackConfigLoader.js b/webpackConfigLoader.js index a8f597412..878897efd 100644 --- a/webpackConfigLoader.js +++ b/webpackConfigLoader.js @@ -1,4 +1,6 @@ /** + * TODO: THIS SHOULD BE REMOVED + * * Allow defaults for the config/webpacker.yml. Thee values in this file MUST match values * in https://github.com/rails/webpacker/blob/master/lib/install/config/webpacker.yml * From 36f2d4485458cd5166a580be203845388a3701cf Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 18 Apr 2020 00:25:54 -1000 Subject: [PATCH 05/10] Switching the name of generator function to render-function --- .gitignore | 3 + CHANGELOG.md | 2 +- CONTRIBUTING.md | 16 +++- README.md | 29 +++---- SUMMARY.md | 2 +- docs/additional-reading/react-helmet.md | 39 ++++++--- docs/additional-reading/react-router.md | 5 +- .../server-rendering-tips.md | 19 +++-- docs/api/javascript-api.md | 6 +- docs/api/redux-store-api.md | 4 +- docs/api/view-helpers-api.md | 9 +-- docs/basics/configuration.md | 2 +- ...d => render-functions-and-railscontext.md} | 81 ++++++++++++++----- docs/basics/upgrading-react-on-rails.md | 30 ++++--- docs/contributor-info/errors-with-hooks.md | 46 +++++++++++ docs/outdated/code-splitting.md | 8 +- .../react_on_rails/dev_tests_generator.rb | 2 +- lib/react_on_rails/helper.rb | 22 ++--- node_package/scripts/symlink-node-package | 2 +- node_package/src/ComponentRegistry.ts | 16 ++-- node_package/src/ReactOnRails.ts | 6 +- node_package/src/clientStartup.ts | 6 +- node_package/src/createReactOutput.ts | 24 +++--- node_package/src/handleError.ts | 14 ++-- ...neratorFunction.ts => isRenderFunction.ts} | 12 +-- node_package/src/types/index.d.ts | 42 +++++----- node_package/tests/ComponentRegistry.test.js | 35 ++++---- node_package/tests/StoreRegistry.test.js | 2 +- ...unction.test.js => renderFunction.test.js} | 24 +++--- .../tests/serverRenderReactComponent.test.js | 8 +- package.json | 3 +- rakelib/dummy_apps.rake | 1 + rakelib/node_package.rake | 4 +- spec/dummy/README.md | 8 +- .../pages/client_side_manual_render.html.erb | 2 +- .../context_function_return_jsx.html.erb | 2 +- ...rred_render_with_server_rendering.html.erb | 2 +- spec/dummy/app/views/pages/react_helmet.erb | 2 +- .../app/views/pages/react_helmet_broken.erb | 4 +- .../pages/server_side_log_throw.html.erb | 2 +- spec/dummy/app/views/shared/_header.erb | 16 ++-- ...sx => ContextFunctionReturnInvalidJSX.jsx} | 9 ++- .../CssModulesImagesFontsExample.jsx | 2 +- .../client/app/packs/clientRegistration.jsx | 4 +- .../client/app/startup/ClientRenderedHtml.jsx | 2 +- .../client/app/startup/HelloWorldApp.jsx | 2 +- .../app/startup/ReactHelmetClientApp.jsx | 6 +- .../app/startup/ReactHelmetServerApp.jsx | 2 +- .../startup/ReactHelmetServerAppBroken.jsx | 2 +- .../client/app/startup/ServerReduxApp.jsx | 2 +- .../client/app/startup/ServerRenderedHtml.jsx | 2 +- .../client/app/startup/ServerRouterApp.jsx | 2 +- .../client/app/startup/serverRegistration.jsx | 4 +- spec/dummy/package.json | 4 +- .../spec/requests/server_render_check_spec.rb | 2 +- spec/dummy/spec/system/integration_spec.rb | 6 +- spec/dummy/spec/system/rails_context_spec.rb | 2 +- spec/dummy/yarn.lock | 8 +- 58 files changed, 384 insertions(+), 239 deletions(-) rename docs/basics/{generator-functions-and-railscontext.md => render-functions-and-railscontext.md} (65%) create mode 100644 docs/contributor-info/errors-with-hooks.md rename node_package/src/{generatorFunction.ts => isRenderFunction.ts} (54%) rename node_package/tests/{generatorFunction.test.js => renderFunction.test.js} (69%) rename spec/dummy/client/app/components/{ContextFunctionReturnJSX.jsx => ContextFunctionReturnInvalidJSX.jsx} (71%) diff --git a/.gitignore b/.gitignore index b8f44148a..badfc5c47 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,6 @@ npm-debug.* /gen-examples .DS_Store + +.yalc +yalc.loc diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc5122d0..f301c2a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ Changes since last non-beta release. In order to solve the issues regarding React Hooks compatability, the number of parameters for functions is used to determine if you have a generator function that will get invoked to return a React component, or you are registering a functional React component. Alternately, you can -set JavaScript property `generatorFunction` on the function for which you want to return to be +set JavaScript property `renderFunction` on the function for which you want to return to be invoked to return the React component. In that case, you won't need to pass any unused params. [PR 1268](https://github.com/shakacode/react_on_rails/pull/1268) by [justin808](https://github.com/justin808) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d86ecada..27f597d91 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,16 +97,26 @@ In addition to testing the Ruby parts out, you can also test the node package pa ```sh cd react_on_rails/ yarn -yarn run build -yarn install-react-on-rails +yarn run build:watch +yalc pubish react-on-rails ``` -Install the local package by using yarn link, like this: +Install the local package by using yalc, like this: ```sh cd spec/dummy +yalc link react-on-rails yarn ``` +Make changes to the node package. + +Then run: + +``` +cd +yalc push +``` + Note, yarn will run the `postinstall` script of `spec/dummy/client` which runs `yarn link` to set up a sym link to the parent package. #### Example: Testing NPM changes with the dummy app diff --git a/README.md b/README.md index ad38f7b74..e6e0b1c3e 100644 --- a/README.md +++ b/README.md @@ -161,13 +161,14 @@ Below is the line where you turn server rendering on by setting `prerender` to t <%= react_component("HelloWorld", props: { name: "Stranger" }) %> ``` -- This is what your HelloWorld.js file might contain. The railsContext is always available for any parameters that you _always_ want available for your React components. It has _nothing_ to do with the concept of the [React Context](https://reactjs.org/docs/context.html). See [Generator Functions and the RailsContext](docs/basics/generator-functions-and-railscontext.md) for more details on this topic. +- This is what your HelloWorld.js file might contain. The railsContext is always available for any parameters that you _always_ want available for your React components. It has _nothing_ to do with the concept of the [React Context](https://reactjs.org/docs/context.html). See [render functions and the RailsContext](docs/basics/render-functions-and-railscontext.md) for more details on this topic. ```js import React from 'react'; export default (props, railsContext) => { - return ( + // Note wrap in a function to make this a React function component + return () => (
    Your locale is {railsContext.i18nLocale}.
    Hello, {props.name}! @@ -227,38 +228,38 @@ 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 Generator Functions +## Specifying Your React Components: Direct or 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 "generator 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 function has the following benefits: -1. You have access to the `railsContext`. See documentation for the railsContext in terms of why you might need it. You **need** a generator function to access the `railsContext`. +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 **generator function** should be JSX or an HTML string. Do not return a +Note, the return value of a **render function** should be JSX or an HTML string. Do not return a function. -ReactOnRails will automatically detect a registered generator function by the fact that the function takes +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 React component, then you need to specify at least a second parameter. This is the `railsContext`. If you're not using this parameter, declare your function with the unused param: ```js const MyComponentGenerator = (props, _railsContext) => { - if (props.print) { - return

    {JSON.stringify(props)}

    ; + // Wrap in a function so this is a React Component + return () =>

    {JSON.stringify(props)}

    ; } - +} ``` -Thus, there is no difference between registering a React function or class Component versus a "generator function." +Thus, there is no difference between registering a React function or class Component versus a "render function." -## react_component_hash for Generator Functions +## react_component_hash for render functions -Another reason to use a generator function is that sometimes in server rendering, specifically with React Router, you need to return the result of calling ReactDOMServer.renderToString(element). You can do this by returning an object with the following shape: { renderedHtml, redirectLocation, error }. Make sure you use this function with `react_component_hash`. +Another reason to use a render function is that sometimes in server rendering, specifically with React Router, you need to return the result of calling ReactDOMServer.renderToString(element). You can do this by returning an object with the following shape: { renderedHtml, redirectLocation, error }. Make sure you use this function with `react_component_hash`. -For server rendering, if you wish to return multiple HTML strings from a generator function, you may return an Object from your generator function with a single top-level property of `renderedHtml`. Inside this Object, place a key called `componentHtml`, along with any other needed keys. An example scenario of this is when you are using side effects libraries like [React Helmet](https://github.com/nfl/react-helmet). Your Ruby code will get this Object as a Hash containing keys componentHtml and any other custom keys that you added: +For server rendering, if you wish to return multiple HTML strings from a render function, you may return an Object from your render function with a single top-level property of `renderedHtml`. Inside this Object, place a key called `componentHtml`, along with any other needed keys. An example scenario of this is when you are using side effects libraries like [React Helmet](https://github.com/nfl/react-helmet). Your Ruby code will get this Object as a Hash containing keys componentHtml and any other custom keys that you added: ```js { renderedHtml: { componentHtml, customKey1, customKey2} } diff --git a/SUMMARY.md b/SUMMARY.md index 19ecf0434..d217ee044 100644 --- a/SUMMARY.md +++ b/SUMMARY.md @@ -9,7 +9,7 @@ + [Client vs. Server Rendering](./docs/basics/client-vs-server-rendering.md) + [React Server Rendering](./docs/basics/react-server-rendering.md) + [Recommended Project Structure](./docs/basics/recommended-project-structure.md) - + [Generator Functions and the RailsContext](docs/basics/generator-functions-and-railscontext.md) + + [Render-Functions and the RailsContext](docs/basics/render-functions-and-railscontext.md) + [Caching and Performance: React on Rails Pro](https://github.com/shakacode/react_on_rails/wiki). + [Deployment](docs/basics/deployment.md). + [React on Rails Internationalization (I18n, localization)](docs/basics/i18n.md) diff --git a/docs/additional-reading/react-helmet.md b/docs/additional-reading/react-helmet.md index 72bb24032..6452e6402 100644 --- a/docs/additional-reading/react-helmet.md +++ b/docs/additional-reading/react-helmet.md @@ -1,12 +1,13 @@ # Using React Helmet to build `` content ## Installation and general usage -See https://github.com/nfl/react-helmet for details. Run `yarn add react-helmet` in your `client` directory to add this package to your application. +See [nfl/react-helmet](https://github.com/nfl/react-helmet) for details on how to use this package. +Run `yarn add react-helmet` to add this package to your application. ## Example Here is what you need to do in order to configure your Rails application to work with **ReactHelmet**. - Create generator function for server rendering like this: + Create a render-function for server rendering like this: ```javascript export default (props, _railsContext) => { @@ -20,17 +21,35 @@ export default (props, _railsContext) => { return { renderedHtml }; }; ``` -You can add more **helmet** properties to result, e.g. **meta**, **base** and so on. See https://github.com/nfl/react-helmet#server-usage. +You can add more **helmet** properties to the result, e.g. **meta**, **base** and so on. See https://github.com/nfl/react-helmet#server-usage. -Use regular component or generator function for client-side: +Use a regular React functional or class component or a render-function for your client-side bundle: ```javascript -export default (props, _railsContext) => ( +// React functional component +export default (props) => ( ); ``` -Put **ReactHelmet** component somewhere in your ``: +Or a render-function. Note you can't return just the JSX (React element), but you need to return +either a React functional or class component. +```javascript +// React functional component +export default (props, railsContext) => ( + () => +); +``` + +Note, this doesn't work, because this function just returns a React element rather than a React component +```javascript +// React functional component +export default (props, railsContext) => ( + +); +``` + +Put the **ReactHelmet** component somewhere in your ``: ```javascript import { Helmet } from 'react-helmet'; @@ -62,9 +81,11 @@ ReactOnRails.register({ ReactHelmetApp }); ``` -Now when the `react_component_hash` helper is called with **"ReactHelmetApp"** as a first argument it will return a hash instead of HTML string: +Now when the `react_component_hash` helper is called with **"ReactHelmetApp"** as a first argument it +will return a hash instead of HTML string. Note, there is no need to specify "prerender" as it would not +make sense to use react_component_hash without server rendering: ```ruby -<% react_helmet_app = react_component_hash("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %> +<% react_helmet_app = react_component_hash("ReactHelmetApp", props: { hello: "world" }, trace: true) %> <% content_for :title do %> <%= react_helmet_app['title'] %> @@ -77,5 +98,3 @@ So now we're able to insert received title tag to our application layout: ```ruby <%= yield(:title) if content_for?(:title) %> ``` - -Note: Use of `react_component` for this functionality is deprecated. Please use `react_component_hash` instead. diff --git a/docs/additional-reading/react-router.md b/docs/additional-reading/react-router.md index 4208f0dc6..2ae767527 100644 --- a/docs/additional-reading/react-router.md +++ b/docs/additional-reading/react-router.md @@ -1,5 +1,8 @@ +_This article needs updating for the latest version of React Router_ + # Using React Router + React on Rails supports the use of React Router. Client-side code doesn't need any special configuration for the React on Rails gem. Implement React Router how you normally would. Note, you might want to avoid using Turbolinks as both Turbolinks and React-Router will be trying to handle the back and forward buttons. If you get this figured out, please do share with the community! Otherwise, you might have to tweak the basic settings for Turbolinks, and this may or may not be worth the effort. If you are working with the HelloWorldApp created by the react_on_rails generator, then the code below corresponds to the module in `client/app/bundles/HelloWorld/startup/HelloWorldApp.jsx`. @@ -36,7 +39,7 @@ For a fleshed out integration of react_on_rails with react-router, check out [Re # Server Rendering Using React Router V4 -Your generator function may not return an object with the property `renderedHtml`. Thus, you call +Your render function may not return an object with the property `renderedHtml`. Thus, you call renderToString() and return an object with this property. This example **only applies to server rendering** and should be only used in the server side bundle. diff --git a/docs/additional-reading/server-rendering-tips.md b/docs/additional-reading/server-rendering-tips.md index 9d4f2e461..64894671f 100644 --- a/docs/additional-reading/server-rendering-tips.md +++ b/docs/additional-reading/server-rendering-tips.md @@ -1,14 +1,19 @@ # Server Rendering Tips -Be sure to use mini_racer. See [issues/428](https://github.com/shakacode/react_on_rails/issues/428) +For the best performance with Server Rendering, consider using [React on Rails Pro] + +Be sure to use mini_racer. See [issues/428](https://github.com/shakacode/react_on_rails/issues/428). + + ## General Tips -- Your code can't reference `document`. Server side JS execution does not have access to `document`, so jQuery and some - other libs won't work in this environment. You can debug this by putting in `console.log` - statements in your code. -- You can conditionally avoid running code that references document by passing in a boolean prop to your top level react - component. Since the passed in props Hash from the view helper applies to client and server side code, the best way to - do this is to use a generator function. +- Your code can't reference `document`. Server side JS execution does not have access to `document`, + so jQuery and some other libs won't work in this environment. You can debug this by putting in + `console.log` statements in your code. +- You can conditionally avoid running code that references document by either checking if `window` + is defined or using the "railsContext" + your top level react component. Since the passed in props Hash from the view helper applies to + client and server side code, the best way to do this is to use a render function. - If you're serious about server rendering, it's worth the effort to have different entry points for client and server rendering. It's worth the extra complexity. The point is that you have separate files for top level client or server side, and you pass some extra option indicating that rendering is happening server side. - You can enable Node.js server rendering via [React on Rails Pro](https://github.com/shakacode/react_on_rails/wiki). diff --git a/docs/api/javascript-api.md b/docs/api/javascript-api.md index 14b62d413..ba4823f5a 100644 --- a/docs/api/javascript-api.md +++ b/docs/api/javascript-api.md @@ -25,10 +25,10 @@ The best source of docs is the main [ReactOnRails.js](https://github.com/shakaco /** * Main entry point to using the react-on-rails npm package. This is how Rails will be able to * find you components for rendering. Components get called with props, or you may use a - * "generator function" to return a React component or an object with the following shape: + * "render function" to return a React component or an object with the following shape: * { renderedHtml, redirectLocation, error }. - * For server rendering, if you wish to return multiple HTML strings from a generator function, - * you may return an Object from your generator function with a single top level property of + * For server rendering, if you wish to return multiple HTML strings from a render function, + * you may return an Object from your render function with a single top level property of * renderedHtml. Inside this Object, place a key called componentHtml, along with any other * needed keys. This is useful when you using side effects libraries like react helmet. * Your Ruby code with get this Object as a Hash containing keys componentHtml and any other diff --git a/docs/api/redux-store-api.md b/docs/api/redux-store-api.md index 593e5b878..ad14b4fc5 100644 --- a/docs/api/redux-store-api.md +++ b/docs/api/redux-store-api.md @@ -1,10 +1,10 @@ # Redux Store -_This redux API is no longer recommended as it prevents dynamic code splitting for performance. Instead, you should use the standard react_component view helper passing in a "generator function."_ +_This redux API is no longer recommended as it prevents dynamic code splitting for performance. Instead, you should use the standard react_component view helper passing in a "render function."_ You don't need to use the `redux_store` api to use redux. This api was setup to support multiple calls to `react_component` on one page that all talk to the same redux store. -If you are only rendering one react component on a page, as is typical to do a "Single Page App" in React, then you should _probably_ pass the props to your React component in a "generator function." +If you are only rendering one react component on a page, as is typical to do a "Single Page App" in React, then you should _probably_ pass the props to your React component in a "render function." Consider using the `redux_store` helper for the two following use cases: diff --git a/docs/api/view-helpers-api.md b/docs/api/view-helpers-api.md index 5d93adb4f..079c167ea 100644 --- a/docs/api/view-helpers-api.md +++ b/docs/api/view-helpers-api.md @@ -22,7 +22,7 @@ Uncommonly used options: id: nil, ``` -- **component_name:** Can be a React component, created using an ES6 class or a generator function that returns a React component (or, only on the server side, an object with shape { redirectLocation, error, renderedHtml }), or a "renderer function" that manually renders a React component to the dom (client side only). +- **component_name:** Can be a React component, created using an ES6 class or a render function that returns a React component (or, only on the server side, an object with shape { redirectLocation, error, renderedHtml }), or a "renderer function" that manually renders a React component to the dom (client side only). All options except `props, id, html_options` will inherit from your `react_on_rails.rb` initializer, as described [here](../basics/configuration.md). - **general options:** - **props:** Ruby Hash which contains the properties to pass to the react object, or a JSON string. If you pass a string, we'll escape it for you. @@ -45,7 +45,7 @@ adding meta-tags to a page. It is exactly like react_component except for the fo 1. `prerender: true` is automatically added to options, as this method doesn't make sense for client only rendering. -2. Your JavaScript generator function for server rendering must return an Object rather than a React Component. +2. Your JavaScript render function for server rendering must return an Object rather than a React Component. 3. Your view code must expect an object and not a string. Here is an example of ERB view code: @@ -73,7 +73,6 @@ export default (props, _railsContext) => { }; return { renderedHtml }; }; - ``` ------------ @@ -99,11 +98,11 @@ You can call `rails_context` or `rails_context(server_side: true|false)` from yo ### Renderer Functions (function that will call ReactDOM.render or ReactDOM.hydrate) -A "renderer function" is a generator function that accepts three arguments (rather than 2): `(props, railsContext, domNodeId) => { ... }`. Instead of returning a React component, a renderer is responsible for installing a callback that will call `ReactDOM.render` (in React 16+, `ReactDOM.hydrate`) to render a React component into the DOM. The "renderer function" is called at the same time the document ready event would instantate the React components into the DOM. +A "renderer function" is a render function that accepts three arguments (rather than 2): `(props, railsContext, domNodeId) => { ... }`. Instead of returning a React component, a renderer is responsible for installing a callback that will call `ReactDOM.render` (in React 16+, `ReactDOM.hydrate`) to render a React component into the DOM. The "renderer function" is called at the same time the document ready event would instantate the React components into the DOM. Why would you want to call `ReactDOM.hydrate` yourself? One possible use case is [code splitting](docs/outdated/code-splitting.md). In a nutshell, you don't want to load the React component on the DOM node yet. So you want to install some handler that will call `ReactDOM.hydrate` at a later time. In the case of code splitting with server rendering, the server rendered code has any async code loaded and used to server render. Thus, the client code must also fully load any asynch code before server rendering. Otherwise, the client code would first render partially, not matching the server rendering, and then a second later, the full code would render, resulting in an unpleasant flashing on the screen. -Renderer functions are not meant to be used on the server since there's no DOM on the server. Instead, use a generator function. Attempting to server render with a renderer function will throw an error. +Renderer functions are not meant to be used on the server since there's no DOM on the server. Instead, use a render function. Attempting to server render with a renderer function will throw an error. ------------ diff --git a/docs/basics/configuration.md b/docs/basics/configuration.md index 1fbb32166..98c6d5c9e 100644 --- a/docs/basics/configuration.md +++ b/docs/basics/configuration.md @@ -199,7 +199,7 @@ Example of a RenderingExtension for custom values in the `rails_context`: module RenderingExtension # Return a Hash that contains custom values from the view context that will get merged with - # the standard rails_context values and passed to all calls to generator functions used by the + # the standard rails_context values and passed to all calls to render functions used by the # react_component and redux_store view helpers def self.custom_context(view_context) { diff --git a/docs/basics/generator-functions-and-railscontext.md b/docs/basics/render-functions-and-railscontext.md similarity index 65% rename from docs/basics/generator-functions-and-railscontext.md rename to docs/basics/render-functions-and-railscontext.md index 404850db8..923584e21 100644 --- a/docs/basics/generator-functions-and-railscontext.md +++ b/docs/basics/render-functions-and-railscontext.md @@ -1,45 +1,80 @@ -# Generator Functions and Rails Context +# Render-Functions and the Rails Context -## Generator Functions +## Render-Functions -When you use a "generator function" to create react components (or renderedHtml on the server), or you used shared redux stores, you get two params passed to your function that creates a React component: +When you use a render-function to create react components (or renderedHtml on the server), or you +used shared redux stores, you get two params passed to your function that creates a React component: 1. `props`: Props that you pass in the view helper of either `react_component` or `redux_store` -2. `railsContext`: Rails contextual information, such as the current pathname. You can customize this in your config file. **Note**: The `railsContext` is not related to the concept of a ["context" for React components](https://facebook.github.io/react/docs/context.html#how-to-use-context). +2. `railsContext`: Rails contextual information, such as the current pathname. You can customize + this in your config file. **Note**: The `railsContext` is not related to the concept of a + ["context" for React components](https://facebook.github.io/react/docs/context.html#how-to-use-context). -This parameters (`props` and `railsContext`) will be the same regardless of either client or server side rendering, except for the key `serverSide` based on whether or not you are server rendering. +These parameters (`props` and `railsContext`) will be the same regardless of either client or server +side rendering, except for the key `serverSide` based on whether or not you are server rendering. -While you could manually configure your Rails code to pass the "`railsContext` information" with the rest of your "props", the `railsContext` is a convenience because it's passed consistently to all invocations of generator functions. +While you could manually configure your Rails code to pass the "`railsContext` information" with +the rest of your "props", the `railsContext` is a convenience because it's passed consistently to +all invocations of render functions. -For example, suppose you create a "generator function" called MyAppComponent. +For example, suppose you create a "render-function" called MyAppComponent. ```js import React from 'react'; const MyAppComponent = (props, railsContext) => ( -
    -

    props are: {JSON.stringify(props)}

    -

    railsContext is: {JSON.stringify(railsContext)} -

    -
    + // NOTE: need to wrap in a function so this is proper React function component that can use + // hooks + + // the props get passed again, but we ignore since we use a closure + // or should we + (_props) => +
    +

    props are: {JSON.stringify(props)}

    +

    railsContext is: {JSON.stringify(railsContext)} +

    +
    ); export default MyAppComponent; ``` +------------------------------ + +_This would be alternate API where you have to call React.createElement and the React on Rails code doesn't do that._ + +```js +import React from 'react'; +const MyAppComponent = (props, railsContext) => ( + // NOTE: need to wrap in a function so this is proper React function component that can use + // hooks + React.createElement( + () => +
    +

    props are: {JSON.stringify(props)}

    +

    railsContext is: {JSON.stringify(railsContext)} +

    +
    , + props) +); +export default MyAppComponent; +``` + +------------------------------------ + *Note: you will get a React browser console warning if you try to serverRender this since the value of `serverSide` will be different for server rendering.* -So if you register your generator function `MyAppComponent`, it will get called like: +So if you register your render-function `MyAppComponent`, it will get called like: ```js reactComponent = MyAppComponent(props, railsContext); ``` -and, similarly, any redux store always initialized with 2 parameters: +and, similarly, any redux store is always initialized with 2 parameters: ```js reduxStore = MyReduxStore(props, railsContext); ``` -Note: you never make these calls. React on Rails makes these calls when it does either client or server rendering. You will define functions that take these 2 params and return a React component or a Redux Store. Naturally, you do not have to use second parameter of the railsContext if you do not need it. +Note: you never make these calls. React on Rails makes these calls when it does either client or server rendering. You will define functions that take these 2 params and return a React component or a Redux Store. Naturally, you do not have to use second parameter of the railsContext if you do not need it. If you don't take a second parameter, then you're probably defining a React function component and you will simply return a React Element, often just JSX. (Note: see below [section](#multiple-react-components-on-a-page-with-one-store) on how to setup redux stores that allow multiple components to talk to the same store.) @@ -76,7 +111,7 @@ Plus, you can add your customizations to this. See "rendering extension" below. ## Rails Context -The `railsContext` is a second param passed to your generator functions for React components. This is in addition to the props that are passed from the `react_component` Rails helper. For example: +The `railsContext` is a second param passed to your render-functions for React components. This is in addition to the props that are passed from the `react_component` Rails helper. For example: ERB view file: @@ -91,7 +126,8 @@ This is what your HelloWorld.js file might contain. The railsContext is always a import React from 'react'; export default (props, railsContext) => { - return ( + // Note, wrap in a function so this is React function component + return () => (
    Your locale is {railsContext.i18nLocale}.
    Hello, {props.name}! @@ -100,15 +136,18 @@ export default (props, railsContext) => { }; ``` -## Why is the railsContext is only passed to generator functions? +## Why is the railsContext only passed to render-functions? + +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: -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 generator 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 "generator 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) => ( - + () => ) export default AppComponentWithRailsContext; ``` @@ -153,7 +192,7 @@ See [spec/dummy/config/initializers/react_on_rails.rb](https://github.com/shakac module RenderingExtension # Return a Hash that contains custom values from the view context that will get merged with - # the standard rails_context values and passed to all calls to generator functions used by the + # the standard rails_context values and passed to all calls to render-functions used by the # react_component and redux_store view helpers def self.custom_context(view_context) { diff --git a/docs/basics/upgrading-react-on-rails.md b/docs/basics/upgrading-react-on-rails.md index 382d19588..28e6558e3 100644 --- a/docs/basics/upgrading-react-on-rails.md +++ b/docs/basics/upgrading-react-on-rails.md @@ -5,37 +5,32 @@ If you would like help in migrating between React on Rails versions or help with We specialize in helping companies to quickly and efficiently move from versions before 9 to current. The older versions use the Rails asset pipeline to package client assets. The current and recommended way is to use Webpack 4 for asset preparation. You may also need help migrating from the `rails/webpacker`'s Webpack configuration to a better setup ready for Server Side Rendering. -## Upgrading to v12 +## Upgrading to v13 * Make sure that are are on a relatively more recent version of rails and webpacker. * Updated API for ReactOnRails.register. In order to solve the issues regarding React Hooks compatability, the number of parameters -for functions is used to determine if you have a generator function that will get invoked to +for functions is used to determine if you have a render function that will get invoked to return a React component, or you are registering a React component defined by a function. -Registered component functions may either do one of the following: +Registered Objects are of the following types: ##### Correct Either of these will work: -1. Take **2 params** and return **a function** that takes zero or one params and returns a - JSX or String. +1. Take **2 params** and return **a React function or class component**. A function component is a function + that takes zero or one params and returns a React Element, like JSX. ```js export default (props, _railsContext) => () => ; ``` -2. Take only zero or one params and you return a JSX literal or String +2. Take only zero or one params and you return a React Element, often JSX. ```js export default (props) => ; ``` -##### Broken, as this function takes two params and it returns a JSX Literal +##### Broken, as this function takes two params and it returns a React Element from a JSX Literal ```js export default (props, _railsContext) => ; ``` -In this example, you need to wrap the `` in a function call, like this: - -```js -export default (props, _railsContext) => () => ; -``` If you make this mistake, you'll get this warning `Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: . Did you accidentally export a JSX literal instead of a component?` @@ -43,12 +38,21 @@ If you make this mistake, you'll get this warning And this error: `react-dom.development.js:23965 Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.` +In this example, you need to wrap the `` in a function call like this which +results in the return value being a React function component. + +```js +export default (props, _railsContext) => () => ; +``` + If you have a pure component, taking one or zero parameters, and you have an unnecessary function -wrapper, then: +wrapper such that you're returning a function rather than a React Element, then: 1. You won't see anything render. 2. You will see this warning in development mode: `Warning: Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it.` +--------- + ## Upgrading rails/webpacker from v3 to v4 ### Custom Webpack build file The default value for `extract_css` is **false** in `config/webpack.yml`. Custom webpack builds should set this value to true or else no CSS link tags are generated. You have a custom webpack build if you are not using [rails/webpacker](https://github.com/rails/webpacker to setup your Webpack configuration. diff --git a/docs/contributor-info/errors-with-hooks.md b/docs/contributor-info/errors-with-hooks.md new file mode 100644 index 000000000..75c420980 --- /dev/null +++ b/docs/contributor-info/errors-with-hooks.md @@ -0,0 +1,46 @@ +# Invalid hook call error + +``` +react.development.js:1465 Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons: +1. You might have mismatching versions of React and the renderer (such as React DOM) +2. You might be breaking the Rules of Hooks +3. You might have more than one copy of React in the same app +See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem. +``` + +The main reason to get this is due to multiple versions of React installed. + +``` +cd +npm ls react + +cd spec/dummy +npm ls react +``` + +For the second one, you might get: + +``` +react_on_rails@ /Users/justin/shakacode/react-on-rails/react_on_rails/spec/dummy +├── react@16.13.1 +└─┬ react-on-rails@12.0.0 -> /Users/justin/shakacode/react-on-rails/react_on_rails invalid + └── react@16.13.1 extraneous + +npm ERR! invalid: react-on-rails@12.0.0 /Users/justin/shakacode/react-on-rails/react_on_rails/spec/dummy/node_modules/react-on-rails +npm ERR! extraneous: react@16.13.1 /Users/justin/shakacode/react-on-rails/react_on_rails/spec/dummy/node_modules/react-on-rails/node_modules/react +``` + +Make sure there is only one version of React installed! + +If you used yarn link, then you'll have two versions of React installed. + +Instead use Yalc. + +``` +cd +yalc publish + +cd spec/dummy +yalc link react-on-rails +``` + diff --git a/docs/outdated/code-splitting.md b/docs/outdated/code-splitting.md index 60653dc6b..1d36ad6b5 100644 --- a/docs/outdated/code-splitting.md +++ b/docs/outdated/code-splitting.md @@ -21,11 +21,11 @@ Let's say you're requesting a page that needs to fetch a code chunk from the ser > (server)
    ; # - # Alternately, you can define the generator function with an additional property - # `.generatorFunction = true`: + # Alternately, you can define the render function with an additional property + # `.renderFunction = true`: # # let MyReactComponentApp = (props) => ; - # MyReactComponent.generatorFunction = true; + # MyReactComponent.renderFunction = true; # # Exposing the react_component_name is necessary to both a plain ReactComponent as well as # a generator: @@ -79,7 +79,7 @@ def react_component(component_name, options = {}) Value: #{server_rendered_html} - If you're trying to use a generator function to return a Hash to your ruby view code, then use + If you're trying to use a render function to return a Hash to your ruby view code, then use react_component_hash instead of react_component and see https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx for an example of the JavaScript code. @@ -93,7 +93,7 @@ def react_component(component_name, options = {}) # It is exactly like react_component except for the following: # 1. prerender: true is automatically added, as this method doesn't make sense for client only # rendering. - # 2. Your JavaScript generator function for server rendering must return an Object rather than a React component. + # 2. Your JavaScript render function for server rendering must return an Object rather than a React component. # 3. Your view code must expect an object and not a string. # # Here is an example of the view code: @@ -124,11 +124,11 @@ def react_component_hash(component_name, options = {}) ) else msg = <<~MSG - Generator function used by react_component_hash for #{component_name} is expected to return + render function used by react_component_hash for #{component_name} is expected to return an Object. See https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx for an example of the JavaScript code. - Note, your generator function must either take 2 params or have the property - `.generatorFunction = true` added to it to distinguish it from a React Function Component. + Note, your render function must either take 2 params or have the property + `.renderFunction = true` added to it to distinguish it from a React Function Component. MSG raise ReactOnRails::Error, msg end @@ -240,7 +240,7 @@ def json_safe_and_pretty(hash_or_string) end # This is the definitive list of the default values used for the rails_context, which is the - # second parameter passed to both component and store generator functions. + # second parameter passed to both component and store render functions. # This method can be called from views and from the controller, as `helpers.rails_context` # # rubocop:disable Metrics/AbcSize diff --git a/node_package/scripts/symlink-node-package b/node_package/scripts/symlink-node-package index 90440edc2..bd080d8c6 100755 --- a/node_package/scripts/symlink-node-package +++ b/node_package/scripts/symlink-node-package @@ -16,6 +16,6 @@ examplesDir=${root_path}/gen-examples/examples if [ -d $examplesDir ] ; then for type in $( ls $examplesDir ); do d=$examplesDir/${type}/client - (cd $d && yarn link react-on-rails) + (cd $d && yalc link react-on-rails) done fi diff --git a/node_package/src/ComponentRegistry.ts b/node_package/src/ComponentRegistry.ts index 231417ceb..39d55bc2c 100644 --- a/node_package/src/ComponentRegistry.ts +++ b/node_package/src/ComponentRegistry.ts @@ -1,5 +1,5 @@ -import type { RegisteredComponent, ComponentOrGeneratorFunction, GeneratorFunction } from './types/index'; -import generatorFunction from './generatorFunction'; +import type { RegisteredComponent, ReactComponentOrRenderFunction, RenderFunction } from './types/index'; +import isRenderFunction from './isRenderFunction'; const registeredComponents = new Map(); @@ -7,7 +7,7 @@ export default { /** * @param components { component1: component1, component2: component2, etc. } */ - register(components: { [id: string]: ComponentOrGeneratorFunction }): void { + register(components: { [id: string]: ReactComponentOrRenderFunction }): void { Object.keys(components).forEach(name => { if (registeredComponents.has(name)) { console.warn('Called register for component that is already registered', name); @@ -18,13 +18,13 @@ export default { throw new Error(`Called register with null component named ${name}`); } - const isGeneratorFunction = generatorFunction(component); - const isRenderer = isGeneratorFunction && (component as GeneratorFunction).length === 3; + const renderFunction = isRenderFunction(component); + const isRenderer = renderFunction && (component as RenderFunction).length === 3; registeredComponents.set(name, { name, component, - generatorFunction: isGeneratorFunction, + renderFunction: renderFunction, isRenderer, }); }); @@ -32,7 +32,7 @@ export default { /** * @param name - * @returns { name, component, generatorFunction, isRenderer } + * @returns { name, component, isRenderFunction, isRenderer } */ get(name: string): RegisteredComponent { if (registeredComponents.has(name)) { @@ -47,7 +47,7 @@ Registered component names include [ ${keys} ]. Maybe you forgot to register the /** * Get a Map containing all registered components. Useful for debugging. * @returns Map where key is the component name and values are the - * { name, component, generatorFunction, isRenderer} + * { name, component, renderFunction, isRenderer} */ components(): Map { return registeredComponents; diff --git a/node_package/src/ReactOnRails.ts b/node_package/src/ReactOnRails.ts index ee702dde2..343cd22e5 100644 --- a/node_package/src/ReactOnRails.ts +++ b/node_package/src/ReactOnRails.ts @@ -15,7 +15,7 @@ import type { RegisteredComponent, RenderParams, ErrorOptions, - ComponentOrGeneratorFunction, + ReactComponentOrRenderFunction, AuthenticityHeaders, StoreGenerator } from './types/index'; @@ -37,7 +37,7 @@ ctx.ReactOnRails = { * find you components for rendering. * @param components (key is component name, value is component) */ - register(components: { [id: string]: ComponentOrGeneratorFunction }): void { + register(components: { [id: string]: ReactComponentOrRenderFunction }): void { ComponentRegistry.register(components); }, @@ -183,7 +183,7 @@ ctx.ReactOnRails = { /** * Get the component that you registered * @param name - * @returns {name, component, generatorFunction, isRenderer} + * @returns {name, component, renderFunction, isRenderer} */ getComponent(name: string): RegisteredComponent { return ComponentRegistry.get(name); diff --git a/node_package/src/clientStartup.ts b/node_package/src/clientStartup.ts index 1f62bfd15..d4c373391 100644 --- a/node_package/src/clientStartup.ts +++ b/node_package/src/clientStartup.ts @@ -4,8 +4,8 @@ import type { ReactOnRails as ReactOnRailsType, RailsContext, RegisteredComponent, - GeneratorFunction, - GeneratorFunctionResult, + RenderFunction, + RenderFunctionResult, } from './types/index'; import createReactOutput from './createReactOutput'; @@ -114,7 +114,7 @@ DELEGATING TO RENDERER ${name} for dom node with id: ${domNodeId} with props, ra props, railsContext); } - (component as GeneratorFunction)(props, railsContext, domNodeId); + (component as RenderFunction)(props, railsContext, domNodeId); return true; } diff --git a/node_package/src/createReactOutput.ts b/node_package/src/createReactOutput.ts index 891b24a85..498c94a13 100644 --- a/node_package/src/createReactOutput.ts +++ b/node_package/src/createReactOutput.ts @@ -2,11 +2,11 @@ import React from 'react'; import type { ServerRenderResult, - CreateParams, ReactComponentVariant, GeneratorFunction, CreateReactOutputResult } from './types/index'; + CreateParams, ReactComponent, RenderFunction, CreateReactOutputResult } from './types/index'; import isServerRenderResult from "./isServerRenderResult"; /** - * Logic to either call the generatorFunction or call React.createElement to get the + * Logic to either call the renderFunction or call React.createElement to get the * React.Component * @param options * @param options.componentObj @@ -24,7 +24,7 @@ export default function createReactOutput({ trace, shouldHydrate, }: CreateParams): CreateReactOutputResult { - const { name, component, generatorFunction } = componentObj; + const { name, component, renderFunction } = componentObj; if (trace) { if (railsContext && railsContext.serverSide) { @@ -38,19 +38,23 @@ export default function createReactOutput({ } } - if (generatorFunction) { + if (renderFunction) { // Let's invoke the function to get the result - let result = (component as GeneratorFunction)(props, railsContext); - if (isServerRenderResult(result as CreateReactOutputResult)) { + let renderFunctionResult = (component as RenderFunction)(props, railsContext); + if (isServerRenderResult(renderFunctionResult as CreateReactOutputResult)) { // 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 (result as ServerRenderResult); + return (renderFunctionResult as ServerRenderResult); } else { // else we'll be calling React.createElement - let reactComponent = result as ReactComponentVariant; + let reactComponent = renderFunctionResult as ReactComponent; + + // TODO: This might be confusing that we're passing props to the function + // immediately afterwards + // Maybe it might be better for the function to return an element. + // The function could call React.createElement return React.createElement(reactComponent, props); } } else { - return React.createElement(component as ReactComponentVariant, props); + return React.createElement(component as ReactComponent, props); } - } diff --git a/node_package/src/handleError.ts b/node_package/src/handleError.ts index 74e9dd29f..6d32ae3b0 100644 --- a/node_package/src/handleError.ts +++ b/node_package/src/handleError.ts @@ -2,19 +2,19 @@ import React from 'react'; import ReactDOMServer from 'react-dom/server'; import type { ErrorOptions } from './types/index'; -function handleGeneratorFunctionIssue(options: {e: Error; name?: string}): string { +function handleRenderFunctionIssue(options: {e: Error; name?: string}): string { const { e, name } = options; let msg = ''; if (name) { const lastLine = - 'A generator function takes a single arg of props (and the location for react-router) ' + + 'A render function takes a single arg of props (and the location for react-router) ' + 'and returns a ReactElement.'; let shouldBeGeneratorError = - `ERROR: ReactOnRails is incorrectly detecting generator function to be false. The React -component '${name}' seems to be a generator function.\n${lastLine}`; + `ERROR: ReactOnRails is incorrectly detecting render function to be false. The React +component '${name}' seems to be a render function.\n${lastLine}`; const reMatchShouldBeGeneratorError = /Can't add property context, object is not extensible/; if (reMatchShouldBeGeneratorError.test(e.message)) { msg += `${shouldBeGeneratorError}\n\n`; @@ -22,8 +22,8 @@ component '${name}' seems to be a generator function.\n${lastLine}`; } shouldBeGeneratorError = - `ERROR: ReactOnRails is incorrectly detecting generatorFunction to be true, but the React -component '${name}' is not a generator function.\n${lastLine}`; + `ERROR: ReactOnRails is incorrectly detecting renderFunction to be true, but the React +component '${name}' is not a render function.\n${lastLine}`; const reMatchShouldNotBeGeneratorError = /Cannot call a class as a function/; @@ -41,7 +41,7 @@ const handleError = (options: ErrorOptions): string => { console.error('Exception in rendering!'); - let msg = handleGeneratorFunctionIssue(options); + let msg = handleRenderFunctionIssue(options); if (jsCode) { console.error(`JS code was: ${jsCode}`); diff --git a/node_package/src/generatorFunction.ts b/node_package/src/isRenderFunction.ts similarity index 54% rename from node_package/src/generatorFunction.ts rename to node_package/src/isRenderFunction.ts index 124bb21ed..f2998ba94 100644 --- a/node_package/src/generatorFunction.ts +++ b/node_package/src/isRenderFunction.ts @@ -1,22 +1,22 @@ // See discussion: // https://discuss.reactjs.org/t/how-to-determine-if-js-object-is-react-component/2825/2 -import { ComponentOrGeneratorFunction, GeneratorFunction } from "./types/index"; +import { ReactComponentOrRenderFunction, RenderFunction } from "./types/index"; /** * Used to determine we'll call be calling React.createElement on the component of if this is a - * generator function used return a function that takes props to return a React element + * render function used return a function that takes props to return a React element * @param component * @returns {boolean} */ -export default function generatorFunction(component: ComponentOrGeneratorFunction): boolean { +export default function isRenderFunction(component: ReactComponentOrRenderFunction): boolean { // No for es5 or es6 React Component if ( - (component as GeneratorFunction).prototype && - (component as GeneratorFunction).prototype.isReactComponent) { + (component as RenderFunction).prototype && + (component as RenderFunction).prototype.isReactComponent) { return false; } - if ((component as GeneratorFunction).generatorFunction) { + if ((component as RenderFunction).renderFunction) { return true; } diff --git a/node_package/src/types/index.d.ts b/node_package/src/types/index.d.ts index fb2c7ad55..d7c67c877 100644 --- a/node_package/src/types/index.d.ts +++ b/node_package/src/types/index.d.ts @@ -2,7 +2,7 @@ import type { ReactElement, Component, FunctionComponent, ComponentClass } from import type { Store } from 'redux'; -type ReactComponentVariant = FunctionComponent | ComponentClass | string; +type ReactComponent = FunctionComponent | ComponentClass | string; interface Params { props?: {}; @@ -39,34 +39,36 @@ export interface RailsContext { httpAcceptLanguage?: string; } -interface ServerRenderResult { - renderedHtml?: string; - redirectLocation?: {pathname: string; search: string}; - routeError?: Error; - error?: Error; -} - type AuthenticityHeaders = {[id: string]: string} & {'X-CSRF-Token': string | null; 'X-Requested-With': string}; type StoreGenerator = (props: {}, railsContext: RailsContext) => Store +interface ServerRenderResult { + renderedHtml?: string; + redirectLocation?: {pathname: string; search: string}; + routeError?: Error; + error?: Error; +} + type CreateReactOutputResult = ServerRenderResult | ReactElement; -type GeneratorFunctionResult = ReactComponentVariant | ServerRenderResult; +type RenderFunctionResult = ReactComponent | ServerRenderResult; -interface GeneratorFunction { - (props?: {}, railsContext?: RailsContext, domNodeId?: string): GeneratorFunctionResult; - generatorFunction?: boolean; +interface RenderFunction { + (props?: {}, railsContext?: RailsContext, domNodeId?: string): RenderFunctionResult; + // We allow specifying that the function is RenderFunction and not a React Function Component + // by setting this property + renderFunction?: boolean; } -type ComponentOrGeneratorFunction = ReactComponentVariant | GeneratorFunction; +type ReactComponentOrRenderFunction = ReactComponent | RenderFunction; export type { // eslint-disable-line import/prefer-default-export - ComponentOrGeneratorFunction, - ReactComponentVariant, + ReactComponentOrRenderFunction, + ReactComponent, AuthenticityHeaders, - GeneratorFunction, - GeneratorFunctionResult, + RenderFunction, + RenderFunctionResult, StoreGenerator, CreateReactOutputResult, ServerRenderResult, @@ -74,8 +76,8 @@ export type { // eslint-disable-line import/prefer-default-export export interface RegisteredComponent { name: string; - component: ComponentOrGeneratorFunction; - generatorFunction: boolean; + component: ReactComponentOrRenderFunction; + renderFunction: boolean; isRenderer: boolean; } @@ -92,7 +94,7 @@ export interface ErrorOptions { } export interface ReactOnRails { - register(components: { [id: string]: ComponentOrGeneratorFunction }): void; + register(components: { [id: string]: ReactComponentOrRenderFunction }): void; registerStore(stores: { [id: string]: Store }): void; getStore(name: string, throwIfMissing: boolean): Store | undefined; setOptions(newOptions: {traceTurbolinks: boolean}): void; diff --git a/node_package/tests/ComponentRegistry.test.js b/node_package/tests/ComponentRegistry.test.js index b7f04be27..191b3db86 100644 --- a/node_package/tests/ComponentRegistry.test.js +++ b/node_package/tests/ComponentRegistry.test.js @@ -11,23 +11,26 @@ import createReactClass from 'create-react-class'; import ComponentRegistry from '../src/ComponentRegistry'; describe('ComponentRegistry', () => { - expect.assertions(9); + beforeEach(() => { + ComponentRegistry.components().clear(); + }); + it('registers and retrieves React function components', () => { expect.assertions(1); const C1 = () =>
    HELLO
    ; ComponentRegistry.register({ C1 }); const actual = ComponentRegistry.get('C1'); - const expected = { name: 'C1', component: C1, generatorFunction: false, isRenderer: false }; + const expected = { name: 'C1', component: C1, renderFunction: false, isRenderer: false }; expect(actual).toEqual(expected); }); - it('registers and retrieves generator function components where property generatorFunction is set and zero params', () => { + it('registers and retrieves render function components where property renderFunction is set and zero params', () => { expect.assertions(1); const C1 = () =>
    HELLO
    ; - C1.generatorFunction = true; + C1.renderFunction = true; ComponentRegistry.register({ C1 }); const actual = ComponentRegistry.get('C1'); - const expected = { name: 'C1', component: C1, generatorFunction: true, isRenderer: false }; + const expected = { name: 'C1', component: C1, renderFunction: true, isRenderer: false }; expect(actual).toEqual(expected); }); @@ -40,7 +43,7 @@ describe('ComponentRegistry', () => { }); ComponentRegistry.register({ C2 }); const actual = ComponentRegistry.get('C2'); - const expected = { name: 'C2', component: C2, generatorFunction: false, isRenderer: false }; + const expected = { name: 'C2', component: C2, renderFunction: false, isRenderer: false }; expect(actual).toEqual(expected); }); @@ -53,7 +56,7 @@ describe('ComponentRegistry', () => { } ComponentRegistry.register({ C3 }); const actual = ComponentRegistry.get('C3'); - const expected = { name: 'C3', component: C3, generatorFunction: false, isRenderer: false }; + const expected = { name: 'C3', component: C3, renderFunction: false, isRenderer: false }; expect(actual).toEqual(expected); }); @@ -62,7 +65,7 @@ describe('ComponentRegistry', () => { const C4 = (a1, a2, a3) => null; ComponentRegistry.register({ C4 }); const actual = ComponentRegistry.get('C4'); - const expected = { name: 'C4', component: C4, generatorFunction: true, isRenderer: true }; + const expected = { name: 'C4', component: C4, renderFunction: true, isRenderer: true }; expect(actual).toEqual(expected); }); @@ -71,33 +74,33 @@ describe('ComponentRegistry', () => { * Thus, tests are cummulative. */ it('registers and retrieves multiple components', () => { - expect.assertions(3); + expect.assertions(4); // Plain react stateless functional components const C5 = () =>
    WHY
    ; const C6 = () =>
    NOW
    ; const C7 = () =>
    NOW
    ; - C7.generatorFunction = true; + C7.renderFunction = true; ComponentRegistry.register({ C5 }); ComponentRegistry.register({ C6 }); ComponentRegistry.register({ C7 }); const components = ComponentRegistry.components(); - expect(components.size).toBe(6); + expect(components.size).toBe(3); expect(components.get('C5')).toEqual({ name: 'C5', component: C5, - generatorFunction: false, + renderFunction: false, isRenderer: false, }); expect(components.get('C6')).toEqual({ name: 'C6', component: C6, - generatorFunction: false, + renderFunction: false, isRenderer: false, }); expect(components.get('C7')).toEqual({ name: 'C7', component: C7, - generatorFunction: true, + renderFunction: true, isRenderer: false, }); }); @@ -112,13 +115,13 @@ describe('ComponentRegistry', () => { expect(components.get('C7')).toEqual({ name: 'C7', component: C7, - generatorFunction: true, + renderFunction: true, isRenderer: false, }); expect(components.get('C8')).toEqual({ name: 'C8', component: C8, - generatorFunction: false, + renderFunction: false, isRenderer: false, }); }); diff --git a/node_package/tests/StoreRegistry.test.js b/node_package/tests/StoreRegistry.test.js index 7a858c5a9..a68affcf5 100644 --- a/node_package/tests/StoreRegistry.test.js +++ b/node_package/tests/StoreRegistry.test.js @@ -35,7 +35,7 @@ describe('', () => { ); }); - it('StoreRegistry registers and retrieves generator function stores', () => { + it('StoreRegistry registers and retrieves render function stores', () => { expect.assertions(2); StoreRegistry.register({ storeGenerator, storeGenerator2 }); const actual = StoreRegistry.getStoreGenerator('storeGenerator'); diff --git a/node_package/tests/generatorFunction.test.js b/node_package/tests/renderFunction.test.js similarity index 69% rename from node_package/tests/generatorFunction.test.js rename to node_package/tests/renderFunction.test.js index e882ba0da..10b9a2fd1 100644 --- a/node_package/tests/generatorFunction.test.js +++ b/node_package/tests/renderFunction.test.js @@ -6,9 +6,9 @@ import React from 'react'; import createReactClass from 'create-react-class'; -import generatorFunction from '../src/generatorFunction'; +import isRenderFunction from '../src/isRenderFunction'; -describe('generatorFunction', () => { +describe('isRenderFunction', () => { expect.assertions(6); it('returns false for a ES5 React Component', () => { expect.assertions(1); @@ -19,7 +19,7 @@ describe('generatorFunction', () => { }, }); - expect(generatorFunction(es5Component)).toBe(false); + expect(isRenderFunction(es5Component)).toBe(false); }); it('returns false for a ES6 React class', () => { @@ -31,7 +31,7 @@ describe('generatorFunction', () => { } } - expect(generatorFunction(ES6Component)).toBe(false); + expect(isRenderFunction(ES6Component)).toBe(false); }); it('returns false for a ES6 React subclass', () => { @@ -49,7 +49,7 @@ describe('generatorFunction', () => { } } - expect(generatorFunction(ES6ComponentChild)).toBe(false); + expect(isRenderFunction(ES6ComponentChild)).toBe(false); }); it('returns false for a stateless functional component with zero params', () => { @@ -57,7 +57,7 @@ describe('generatorFunction', () => { const pureComponent = () =>

    Hello

    ; - expect(generatorFunction(pureComponent)).toBe(false); + expect(isRenderFunction(pureComponent)).toBe(false); }); it('returns false for a stateless functional component with one param', () => { @@ -67,16 +67,16 @@ describe('generatorFunction', () => { const pureComponent = (props) =>

    {props.title}

    ; /* eslint-enable react/prop-types */ - expect(generatorFunction(pureComponent)).toBe(false); + expect(isRenderFunction(pureComponent)).toBe(false); }); - it('returns true for a generator function (containing two params)', () => { + it('returns true for a render function (containing two params)', () => { expect.assertions(1); - const foobarComponent = () =>
    Component for Generator Function
    ; - const foobarGeneratorFunction = (_props, _railsContext) => foobarComponent; + const foobarComponent = () =>
    Component for render function
    ; + const foobarrenderFunction = (_props, _railsContext) => foobarComponent; - expect(generatorFunction(foobarGeneratorFunction)).toBe(true); + expect(isRenderFunction(foobarrenderFunction)).toBe(true); }); it('returns false for simple object', () => { @@ -87,6 +87,6 @@ describe('generatorFunction', () => { return 'world'; }, }; - expect(generatorFunction(foobarComponent)).toBe(false); + expect(isRenderFunction(foobarComponent)).toBe(false); }); }); diff --git a/node_package/tests/serverRenderReactComponent.test.js b/node_package/tests/serverRenderReactComponent.test.js index 89b6a97e9..3865b4077 100644 --- a/node_package/tests/serverRenderReactComponent.test.js +++ b/node_package/tests/serverRenderReactComponent.test.js @@ -5,9 +5,13 @@ import React from 'react'; import serverRenderReactComponent from '../src/serverRenderReactComponent'; import ComponentStore from '../src/ComponentRegistry'; +import ComponentRegistry from "../src/ComponentRegistry"; describe('serverRenderReactComponent', () => { - expect.assertions(7); + beforeEach(() => { + ComponentRegistry.components().clear(); + }); + it('serverRenderReactComponent renders a registered component', () => { expect.assertions(2); const X1 = () =>
    HELLO
    ; @@ -44,7 +48,7 @@ describe('serverRenderReactComponent', () => { it('serverRenderReactComponent renders html', () => { expect.assertions(2); const expectedHtml = '
    Hello
    '; - const X3 = () => ({ renderedHtml: expectedHtml }); + const X3 = (props, _railsContext) => ({ renderedHtml: expectedHtml }); ComponentStore.register({ X3 }); diff --git a/package.json b/package.json index f7fac9d11..b8bc48394 100644 --- a/package.json +++ b/package.json @@ -76,8 +76,7 @@ "release:minor": "node_package/scripts/release minor", "release:major": "node_package/scripts/release major", "dummy:install": "rake dummy_apps", - "dummy:spec": "rake run_rspec:dummy", - "install-react-on-rails": "yarn link" + "dummy:spec": "rake run_rspec:dummy" }, "repository": { "type": "git", diff --git a/rakelib/dummy_apps.rake b/rakelib/dummy_apps.rake index 3b9453e2f..2fd796508 100644 --- a/rakelib/dummy_apps.rake +++ b/rakelib/dummy_apps.rake @@ -8,6 +8,7 @@ namespace :dummy_apps do task :yarn_install do yarn_install_cmd = "yarn install --mutex network" sh_in_dir(dummy_app_dir, yarn_install_cmd) + sh_in_dir(dummy_app_dir, "yalc link react-on-rails") end task dummy_app: [:yarn_install] do diff --git a/rakelib/node_package.rake b/rakelib/node_package.rake index c28797553..4eeb2b234 100644 --- a/rakelib/node_package.rake +++ b/rakelib/node_package.rake @@ -6,8 +6,8 @@ namespace :node_package do include ReactOnRails::TaskHelpers task :build do - puts "Building Node Package and running 'yarn link'" - sh "yarn run build && yarn link" + puts "Building Node Package and running 'yalc link'" + sh "yarn run build && yalc publish" end end diff --git a/spec/dummy/README.md b/spec/dummy/README.md index 72492bbf7..c5a44e578 100644 --- a/spec/dummy/README.md +++ b/spec/dummy/README.md @@ -1,15 +1,15 @@ Using NPM for react_on_rails -* Use 'yarn link' to hook up the spec/dummy/client/node_modules to the top level +* Use 'yalc link' to hook up the spec/dummy/client/node_modules to the top level * Be sure to install yarn dependencies in spec/dummy/client ## Setup yarn link ```sh cd react_on_rails -yarn link -cd spec/dummy/client -yarn link react-on-rails +yalc publish +cd spec/dummy +yalc link react-on-rails ``` ## Run yarn if not done yet diff --git a/spec/dummy/app/views/pages/client_side_manual_render.html.erb b/spec/dummy/app/views/pages/client_side_manual_render.html.erb index 55ec2d935..bdf31c9ba 100644 --- a/spec/dummy/app/views/pages/client_side_manual_render.html.erb +++ b/spec/dummy/app/views/pages/client_side_manual_render.html.erb @@ -5,7 +5,7 @@

    Using Renderer Functions to Manually Render Your App

    • -

      Use react_component the same way you would if you were registering a generator function or component:

      +

      Use react_component the same way you would if you were registering a render function or component:

         <%%= react_component("ManualRenderApp", props: { prerender: false }, prerender: false) %>
           
      diff --git a/spec/dummy/app/views/pages/context_function_return_jsx.html.erb b/spec/dummy/app/views/pages/context_function_return_jsx.html.erb index b4d71aacc..9970c01a3 100644 --- a/spec/dummy/app/views/pages/context_function_return_jsx.html.erb +++ b/spec/dummy/app/views/pages/context_function_return_jsx.html.erb @@ -1,4 +1,4 @@ -<%= react_component("ContextFunctionReturnJSX", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %> +<%= react_component("ContextFunctionReturnInvalidJSX", props: @app_props_server_render, prerender: false, trace: true, id: "HelloWorld-react-component-0") %>

      Example of error from failing to wrap the result in a function.

      diff --git a/spec/dummy/app/views/pages/deferred_render_with_server_rendering.html.erb b/spec/dummy/app/views/pages/deferred_render_with_server_rendering.html.erb index 1d0625505..60bff44b4 100644 --- a/spec/dummy/app/views/pages/deferred_render_with_server_rendering.html.erb +++ b/spec/dummy/app/views/pages/deferred_render_with_server_rendering.html.erb @@ -20,7 +20,7 @@
    • -

      Don't register rendering functions on the server side. Instead, create a generator function for use on the server:

      +

      Don't register rendering functions on the server side. Instead, create a render function for use on the server:

         import DeferredRenderApp from './DeferredRenderAppServer';
       
      diff --git a/spec/dummy/app/views/pages/react_helmet.erb b/spec/dummy/app/views/pages/react_helmet.erb
      index 0b4174f19..ccedc3c39 100644
      --- a/spec/dummy/app/views/pages/react_helmet.erb
      +++ b/spec/dummy/app/views/pages/react_helmet.erb
      @@ -13,6 +13,6 @@
       
       
      -This page demonstrates a generator function that returns htmlResult as an object +This page demonstrates a render function that returns htmlResult as an object with HTML strings on the server side. It is useful to manipulating <head> content. Check out the page title! diff --git a/spec/dummy/app/views/pages/react_helmet_broken.erb b/spec/dummy/app/views/pages/react_helmet_broken.erb index 11aa267f3..62ad46790 100644 --- a/spec/dummy/app/views/pages/react_helmet_broken.erb +++ b/spec/dummy/app/views/pages/react_helmet_broken.erb @@ -13,9 +13,9 @@
      -This page demonstrates an improperly defined generator function. +This page demonstrates an improperly defined render function. -

      Here's the error message when you forget to mark the function as a generator function:

      +

      Here's the error message when you forget to mark the function as a render function:

         Objects are not valid as a React child (found: object with keys {renderedHtml}). If you meant to render a collection of children, use an array instead.\n    in Unknown
      diff --git a/spec/dummy/app/views/pages/server_side_log_throw.html.erb b/spec/dummy/app/views/pages/server_side_log_throw.html.erb
      index 01f98aeac..ff02ce480 100644
      --- a/spec/dummy/app/views/pages/server_side_log_throw.html.erb
      +++ b/spec/dummy/app/views/pages/server_side_log_throw.html.erb
      @@ -1,5 +1,5 @@
       
      -

      React Rails Server Rendering with throw from "generator function"

      +

      React Rails Server Rendering with throw from "render function"

      This example demonstrates server side logging and error handling.
      Open up your browser console and see the messages.
      diff --git a/spec/dummy/app/views/shared/_header.erb b/spec/dummy/app/views/shared/_header.erb index e5e5c2b2e..7fe3ea199 100644 --- a/spec/dummy/app/views/shared/_header.erb +++ b/spec/dummy/app/views/shared/_header.erb @@ -2,6 +2,9 @@

    • <%= link_to "One Page with Many Examples at Once", root_path %>
    • +
    • + <%= link_to "Hello World Component Server Rendered Hooks", server_side_hello_world_hooks_path %> +
    • <%= link_to "Hello World Component Client Rendered", client_side_hello_world_path %>
    • @@ -32,9 +35,6 @@
    • <%= link_to "Hello World Component Server Rendered", server_side_hello_world_path %>
    • -
    • - <%= link_to "Hello World Component Server Rendered Hooks", server_side_hello_world_hooks_path %> -
    • <%= link_to "Hello World Component Server Rendered, with extra options", server_side_hello_world_with_options_path %>
    • @@ -42,10 +42,10 @@ <%= link_to "Hello World Component Server Rendered ES5", server_side_hello_world_es5_path %>
    • - <%= link_to "Redux App Server Rendered (Generator Function)", server_side_redux_app_path %> + <%= link_to "Redux App Server Rendered (render function)", server_side_redux_app_path %>
    • - <%= link_to "Redux App Server Rendered (Generator Function) With Fragment Caching", server_side_redux_app_cached_path %> + <%= link_to "Redux App Server Rendered (render function) With Fragment Caching", server_side_redux_app_cached_path %>
    • <%= link_to "Client Logging and Throw Error Handling", client_side_log_throw_path %> @@ -84,13 +84,13 @@ <%= link_to "Turbolinks Cache Disabled Example", turbolinks_cache_disabled_path %>
    • - <%= link_to "Generator function returns object with renderedHtml", rendered_html_path %> + <%= link_to "render function returns object with renderedHtml", rendered_html_path %>
    • - <%= link_to "Generator function returns object with renderedHtml as another object (react-helmet)", react_helmet_path %> + <%= link_to "render function returns object with renderedHtml as another object (react-helmet)", react_helmet_path %>
    • - <%= link_to "Improperly defined generator function (react-helmet)", react_helmet_broken_path %> + <%= link_to "Improperly defined render function (react-helmet)", react_helmet_broken_path %>
    • <%= link_to "Image Example", image_example_path %> diff --git a/spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx b/spec/dummy/client/app/components/ContextFunctionReturnInvalidJSX.jsx similarity index 71% rename from spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx rename to spec/dummy/client/app/components/ContextFunctionReturnInvalidJSX.jsx index 0a5962fa5..7bc5343f8 100644 --- a/spec/dummy/client/app/components/ContextFunctionReturnJSX.jsx +++ b/spec/dummy/client/app/components/ContextFunctionReturnInvalidJSX.jsx @@ -3,7 +3,7 @@ import React, { useState } from 'react'; import css from './HelloWorld.scss'; import RailsContext from './RailsContext'; -const ContextFunctionReturnJSX = (props, railsContext) => ( +const ContextFunctionReturnInvalidJSX = (props, railsContext) => ( <>

      Hello, {props.name}!

      Rails Context :

      @@ -13,7 +13,10 @@ const ContextFunctionReturnJSX = (props, railsContext) => ( /* Wrapping in a function would be correct in this case, since two params are passed to the registered function: -const ContextFunctionReturnJSX = (props, railsContext) => () => ( + + This code should have been written like: + +const ContextFunctionReturnInvalidJSX = (props, railsContext) => () => ( <>

      Hello, {props.name}!

      Rails Context :

      @@ -22,4 +25,4 @@ const ContextFunctionReturnJSX = (props, railsContext) => () => ( ); */ -export default ContextFunctionReturnJSX; +export default ContextFunctionReturnInvalidJSX; diff --git a/spec/dummy/client/app/components/CssModulesImagesFontsExample.jsx b/spec/dummy/client/app/components/CssModulesImagesFontsExample.jsx index 59a38b302..53f6cfdca 100644 --- a/spec/dummy/client/app/components/CssModulesImagesFontsExample.jsx +++ b/spec/dummy/client/app/components/CssModulesImagesFontsExample.jsx @@ -3,7 +3,7 @@ import React from 'react'; import styles from './CssModulesImagesFontsExample.scss'; -export default (_props, _railsContext) => ( +export default (_props) => (

      This should be open sans light

      diff --git a/spec/dummy/client/app/packs/clientRegistration.jsx b/spec/dummy/client/app/packs/clientRegistration.jsx index 797051e54..0ea7e0e2c 100644 --- a/spec/dummy/client/app/packs/clientRegistration.jsx +++ b/spec/dummy/client/app/packs/clientRegistration.jsx @@ -6,7 +6,7 @@ import ReactOnRails from 'react-on-rails'; import HelloWorld from '../components/HelloWorld'; import HelloWorldHooks from '../components/HelloWorldHooks'; import HelloWorldHooksContext from '../components/HelloWorldHooksContext'; -import ContextFunctionReturnJSX from '../components/ContextFunctionReturnJSX'; +import ContextFunctionReturnInvalidJSX from '../components/ContextFunctionReturnInvalidJSX'; import PureComponentWrappedInFunction from '../components/PureComponentWrappedInFunction'; import HelloWorldWithLogAndThrow from '../components/HelloWorldWithLogAndThrow'; @@ -61,7 +61,7 @@ ReactOnRails.register({ SetTimeoutLoggingApp, HelloWorldHooks, HelloWorldHooksContext, - ContextFunctionReturnJSX, + ContextFunctionReturnJSX: ContextFunctionReturnInvalidJSX, PureComponentWrappedInFunction, }); diff --git a/spec/dummy/client/app/startup/ClientRenderedHtml.jsx b/spec/dummy/client/app/startup/ClientRenderedHtml.jsx index 90269312f..5caffd36e 100644 --- a/spec/dummy/client/app/startup/ClientRenderedHtml.jsx +++ b/spec/dummy/client/app/startup/ClientRenderedHtml.jsx @@ -9,7 +9,7 @@ import EchoProps from '../components/EchoProps'; * React will see that the state is the same and not do anything. * Note, this is imported as "HelloWorldApp" by "clientRegistration.jsx" * - * Note, this is a fictional example, as you'd only use a generator function if you wanted to run + * Note, this is a fictional example, as you'd only use a render function if you wanted to run * some extra code, such as setting up Redux and React-Router. */ diff --git a/spec/dummy/client/app/startup/HelloWorldApp.jsx b/spec/dummy/client/app/startup/HelloWorldApp.jsx index 8a031054b..77fb03f1c 100644 --- a/spec/dummy/client/app/startup/HelloWorldApp.jsx +++ b/spec/dummy/client/app/startup/HelloWorldApp.jsx @@ -8,7 +8,7 @@ import HelloWorld from '../components/HelloWorld'; * React will see that the state is the same and not do anything. * Note, this is imported as "HelloWorldApp" by "clientRegistration.jsx" * - * Note, this is a fictional example, as you'd only use a generator function if you wanted to run + * Note, this is a fictional example, as you'd only use a render function if you wanted to run * some extra code, such as setting up Redux and React-Router. */ export default (props) => ; diff --git a/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx b/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx index dbfdbecdc..9bb73634c 100644 --- a/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx +++ b/spec/dummy/client/app/startup/ReactHelmetClientApp.jsx @@ -5,9 +5,9 @@ import ReactHelmet from '../components/ReactHelmet'; // This works fine, React functional component: // export default (props) => ; -export default (props, _railsContext) => ; +export default (props) => ; -// Note, the server side has to be a generator function +// Note, the server side has to be a render function -// If you want a generatorFunction, return a ReactComponent +// If you want a renderFunction, return a ReactComponent // export default (props, _railsContext) => () => ; diff --git a/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx b/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx index aec7088a1..44e4152a1 100644 --- a/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx +++ b/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx @@ -13,7 +13,7 @@ import ReactHelmet from '../components/ReactHelmet'; * component markup must go under "componentHtml" key. * * Note that the function takes 2 params to identify this as a generator fuction. Alternately, - * the function could get the property of `.generatorFunction = true` added to it. + * the function could get the property of `.renderFunction = true` added to it. */ export default (props, _railsContext) => { const componentHtml = renderToString(); diff --git a/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx b/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx index 39ae45d5e..881e4c8b8 100644 --- a/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx +++ b/spec/dummy/client/app/startup/ReactHelmetServerAppBroken.jsx @@ -15,7 +15,7 @@ import ReactHelmet from '../components/ReactHelmet'; * component markup must go under "componentHtml" key. * * Note that the function should take 2 params to identify this as a generator fuction. - * Alternately, the function could get the property of `.generatorFunction = true` added to it. + * Alternately, the function could get the property of `.renderFunction = true` added to it. */ export default (props) => { const componentHtml = renderToString(); diff --git a/spec/dummy/client/app/startup/ServerReduxApp.jsx b/spec/dummy/client/app/startup/ServerReduxApp.jsx index 0f8e59d12..bdcc31796 100644 --- a/spec/dummy/client/app/startup/ServerReduxApp.jsx +++ b/spec/dummy/client/app/startup/ServerReduxApp.jsx @@ -32,7 +32,7 @@ export default (props, railsContext) => { // Provider uses the this.props.children, so we're not typical React syntax. // This allows redux to add additional props to the HelloWorldContainer. - return ( + return () => ( diff --git a/spec/dummy/client/app/startup/ServerRenderedHtml.jsx b/spec/dummy/client/app/startup/ServerRenderedHtml.jsx index 543407a82..18d824c63 100644 --- a/spec/dummy/client/app/startup/ServerRenderedHtml.jsx +++ b/spec/dummy/client/app/startup/ServerRenderedHtml.jsx @@ -7,7 +7,7 @@ import EchoProps from '../components/EchoProps'; * Export a function that takes the props and returns an object with { renderedHtml } * Note, this is imported as "RenderedHtml" by "serverRegistration.jsx" * - * Note, this is a fictional example, as you'd only use a generator function if you wanted to run + * Note, this is a fictional example, as you'd only use a render function if you wanted to run * some extra code, such as setting up Redux and React-Router. * * And the use of renderToString would probably be done with react-router v4 diff --git a/spec/dummy/client/app/startup/ServerRouterApp.jsx b/spec/dummy/client/app/startup/ServerRouterApp.jsx index d499c12d0..6eed0ce69 100644 --- a/spec/dummy/client/app/startup/ServerRouterApp.jsx +++ b/spec/dummy/client/app/startup/ServerRouterApp.jsx @@ -25,5 +25,5 @@ export default (_props, railsContext) => { } // Important that you don't do this if you are redirecting or have an error. - return ; + return () => ; }; diff --git a/spec/dummy/client/app/startup/serverRegistration.jsx b/spec/dummy/client/app/startup/serverRegistration.jsx index c45a8fcf0..199938361 100644 --- a/spec/dummy/client/app/startup/serverRegistration.jsx +++ b/spec/dummy/client/app/startup/serverRegistration.jsx @@ -13,7 +13,7 @@ import HelloWorldES5 from '../components/HelloWorldES5'; import HelloWorldRehydratable from '../components/HelloWorldRehydratable'; import HelloWorldWithLogAndThrow from '../components/HelloWorldWithLogAndThrow'; -// Generator function +// render function import HelloWorldApp from '../startup/HelloWorldApp'; import BrokenApp from '../startup/BrokenApp'; @@ -41,7 +41,7 @@ import RenderedHtml from '../startup/ServerRenderedHtml'; import ReactHelmetApp from '../startup/ReactHelmetServerApp'; // Broken server render since ReactHelmetServerAppBroken is not properly defined -// to be a generatorFunction +// to be a renderFunction import ReactHelmetAppBroken from '../startup/ReactHelmetServerAppBroken'; // Demonstrate using Images diff --git a/spec/dummy/package.json b/spec/dummy/package.json index a9e7f9cdf..97770c1de 100644 --- a/spec/dummy/package.json +++ b/spec/dummy/package.json @@ -44,7 +44,7 @@ "react-dom": "^16.8.6", "react-helmet": "^6.0.0-beta.2", "react-hot-loader": "^4.12.12", - "react-on-rails": "13.0.0", + "react-on-rails": "11.3.0", "react-proptypes": "^1.0.0", "react-redux": "^5.0.5", "react-router": "^3.2.1", @@ -80,7 +80,7 @@ "fs": false }, "scripts": { - "postinstall": "yarn link react-on-rails", + "yalc-postinstall": "yalc link react-on-rails", "lint": "cd client && yarn run lint", "test": "yarn run build:test && yarn run lint && rspec", "build:test": "rm -rf public/webpack/test && RAILS_ENV=test NODE_ENV=test bin/webpack", diff --git a/spec/dummy/spec/requests/server_render_check_spec.rb b/spec/dummy/spec/requests/server_render_check_spec.rb index 4452fd412..441fd8c54 100644 --- a/spec/dummy/spec/requests/server_render_check_spec.rb +++ b/spec/dummy/spec/requests/server_render_check_spec.rb @@ -148,7 +148,7 @@ def do_request(path) end end - context "generator function" do + context "render function" do it "matches expected values" do do_request(server_side_redux_app_path) check_match("server_side_redux_app", "ReduxApp") diff --git a/spec/dummy/spec/system/integration_spec.rb b/spec/dummy/spec/system/integration_spec.rb index b11b26b9c..b95e35f0c 100644 --- a/spec/dummy/spec/system/integration_spec.rb +++ b/spec/dummy/spec/system/integration_spec.rb @@ -68,7 +68,7 @@ def finished_all_ajax_requests? context "Simple stateless component" do include_examples "React Component", "div#HelloWorldApp-react-component-6" end - context "Generator function that takes props" do + context "render function that takes props" do include_examples "React Component", "div#HelloWorldApp-react-component-7" end end @@ -199,7 +199,7 @@ def finished_all_ajax_requests? end end -feature "renderedHtml from generator function", :js, type: :system do +feature "renderedHtml from render function", :js, type: :system do subject { page } background { visit "/rendered_html" } scenario "renderedHtml should not have any errors" do @@ -234,7 +234,7 @@ def finished_all_ajax_requests? end end -feature "generator function returns renderedHtml as an object with additional HTML markups" do +feature "render function returns renderedHtml as an object with additional HTML markups" do shared_examples "renderedHtmls should not have any errors and set correct page title" do subject { page } background { visit react_helmet_path } diff --git a/spec/dummy/spec/system/rails_context_spec.rb b/spec/dummy/spec/system/rails_context_spec.rb index e324ec005..afb73da22 100644 --- a/spec/dummy/spec/system/rails_context_spec.rb +++ b/spec/dummy/spec/system/rails_context_spec.rb @@ -62,7 +62,7 @@ "ReduxSharedStoreApp") end - context "generator function for component" do + context "render function for component" do include_examples("railsContext", "server_side_redux_app", "ReduxApp") diff --git a/spec/dummy/yarn.lock b/spec/dummy/yarn.lock index decdace16..fa350e380 100644 --- a/spec/dummy/yarn.lock +++ b/spec/dummy/yarn.lock @@ -8370,10 +8370,10 @@ react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA== -react-on-rails@11.2.2: - version "11.2.2" - resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-11.2.2.tgz#e5cc062e41832f548d9d40dfc2aebd586b932f95" - integrity sha512-ZTS69giZDxVxwnraFNKujHpzK9GO5be+ISyGw5v3rKs4VlLKJ7BTVDtT22Fn0iEMe6x817s/cwGwepkAEEwA9A== +react-on-rails@11.3.0: + version "11.3.0" + resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-11.3.0.tgz#edebfd4b98f1fa110e4d5ce92ef699d7dccf8fb2" + integrity sha512-3eyoZe+tysxdMy7IZ7Ca6gar268iX1NOJy7hJH8CW9dfNIFWbeWXTVa3G91wrt5xo2toWZLCoj51WcBWw0Rm3Q== dependencies: "@babel/runtime-corejs2" "^7.0.0" From c2c68239729922f71ec7fcd44576788e23772884 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 18 Apr 2020 00:31:39 -1000 Subject: [PATCH 06/10] Lint fixes --- .prettierignore | 1 - lib/react_on_rails/helper.rb | 24 +++++++++---------- .../tests/serverRenderReactComponent.test.js | 11 ++++----- spec/dummy/.prettierignore | 1 + 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/.prettierignore b/.prettierignore index 8307ccfc4..0a9e89c0a 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,6 +1,5 @@ node_modules/ package.json tmp/ -public/webpack/ coverage/ **/app/assets/webpack/ diff --git a/lib/react_on_rails/helper.rb b/lib/react_on_rails/helper.rb index f2d6a8357..8490e4832 100644 --- a/lib/react_on_rails/helper.rb +++ b/lib/react_on_rails/helper.rb @@ -66,23 +66,23 @@ def react_component(component_name, options = {}) ) elsif server_rendered_html.is_a?(Hash) msg = <<~MSG - Use react_component_hash (not react_component) to return a Hash to your ruby view code. See - https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx - for an example of the necessary javascript configuration. + Use react_component_hash (not react_component) to return a Hash to your ruby view code. See + https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx + for an example of the necessary javascript configuration. MSG raise ReactOnRails::Error, msg else class_name = server_rendered_html.class.name msg = <<~MSG - ReactOnRails: server_rendered_html is expected to be a String or Hash for #{component_name}. - Type is #{class_name} - Value: - #{server_rendered_html} - - If you're trying to use a render function to return a Hash to your ruby view code, then use - react_component_hash instead of react_component and see - https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx - for an example of the JavaScript code. + ReactOnRails: server_rendered_html is expected to be a String or Hash for #{component_name}. + Type is #{class_name} + Value: + #{server_rendered_html} + + If you're trying to use a render function to return a Hash to your ruby view code, then use + react_component_hash instead of react_component and see + https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ReactHelmetServerApp.jsx + for an example of the JavaScript code. MSG raise ReactOnRails::Error, msg end diff --git a/node_package/tests/serverRenderReactComponent.test.js b/node_package/tests/serverRenderReactComponent.test.js index 3865b4077..330f8a822 100644 --- a/node_package/tests/serverRenderReactComponent.test.js +++ b/node_package/tests/serverRenderReactComponent.test.js @@ -4,8 +4,7 @@ import React from 'react'; import serverRenderReactComponent from '../src/serverRenderReactComponent'; -import ComponentStore from '../src/ComponentRegistry'; -import ComponentRegistry from "../src/ComponentRegistry"; +import ComponentRegistry from '../src/ComponentRegistry'; describe('serverRenderReactComponent', () => { beforeEach(() => { @@ -15,7 +14,7 @@ describe('serverRenderReactComponent', () => { it('serverRenderReactComponent renders a registered component', () => { expect.assertions(2); const X1 = () =>
      HELLO
      ; - ComponentStore.register({ X1 }); + ComponentRegistry.register({ X1 }); const { html, hasErrors } = JSON.parse( serverRenderReactComponent({ name: 'X1', domNodeId: 'myDomId', trace: false }), @@ -32,7 +31,7 @@ describe('serverRenderReactComponent', () => { throw new Error('XYZ'); }; - ComponentStore.register({ X2 }); + ComponentRegistry.register({ X2 }); // Not testing the consoleReplayScript, as handleError is putting the console to the test // runner log. @@ -50,7 +49,7 @@ describe('serverRenderReactComponent', () => { const expectedHtml = '
      Hello
      '; const X3 = (props, _railsContext) => ({ renderedHtml: expectedHtml }); - ComponentStore.register({ X3 }); + ComponentRegistry.register({ X3 }); const { html, hasErrors, renderedHtml } = JSON.parse( serverRenderReactComponent({ name: 'X3', domNodeId: 'myDomId', trace: false }), @@ -63,7 +62,7 @@ describe('serverRenderReactComponent', () => { it('serverRenderReactComponent renders an error if attempting to render a renderer', () => { expect.assertions(1); const X3 = (a1, a2, a3) => null; - ComponentStore.register({ X3 }); + ComponentRegistry.register({ X3 }); const { html } = JSON.parse( serverRenderReactComponent({ name: 'X3', domNodeId: 'myDomId', trace: false }), diff --git a/spec/dummy/.prettierignore b/spec/dummy/.prettierignore index 3c3629e64..8a62d1007 100644 --- a/spec/dummy/.prettierignore +++ b/spec/dummy/.prettierignore @@ -1 +1,2 @@ node_modules +public/webpack/ From e9383205f684ad0261632b53fc6fcebb61ec4a56 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 18 Apr 2020 00:33:05 -1000 Subject: [PATCH 07/10] gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index badfc5c47..971e77111 100644 --- a/.gitignore +++ b/.gitignore @@ -33,4 +33,4 @@ npm-debug.* .DS_Store .yalc -yalc.loc +yalc.lock From 64fa79b3c062566ab24169bb697519b8a09ccebe Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 18 Apr 2020 00:36:46 -1000 Subject: [PATCH 08/10] Update travis to have yalc --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8f97b1257..c1d1faf7d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,6 +39,7 @@ install: - node -v - travis_retry npm i -g yarn - travis_retry bundle install + - travis_retry yarn global add yalc - travis_retry yarn - travis_retry yarn run build - bundle exec rake prepare_for_ci From 7dd049cc0c86c31147947f9221256a156372d00a Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Fri, 24 Apr 2020 21:43:28 -0500 Subject: [PATCH 09/10] possible ci fixes --- spec/dummy/client/app/startup/DeferredRenderAppServer.jsx | 2 +- spec/dummy/spec/system/integration_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/dummy/client/app/startup/DeferredRenderAppServer.jsx b/spec/dummy/client/app/startup/DeferredRenderAppServer.jsx index 46128ddb3..655498f3a 100644 --- a/spec/dummy/client/app/startup/DeferredRenderAppServer.jsx +++ b/spec/dummy/client/app/startup/DeferredRenderAppServer.jsx @@ -34,7 +34,7 @@ const DeferredRenderAppServer = (_props, railsContext) => { return { error, redirectLocation }; } - return ; + return () => ; }; export default DeferredRenderAppServer; diff --git a/spec/dummy/spec/system/integration_spec.rb b/spec/dummy/spec/system/integration_spec.rb index b95e35f0c..5715212fb 100644 --- a/spec/dummy/spec/system/integration_spec.rb +++ b/spec/dummy/spec/system/integration_spec.rb @@ -66,10 +66,10 @@ def finished_all_ajax_requests? context "React Hooks" do context "Simple stateless component" do - include_examples "React Component", "div#HelloWorldApp-react-component-6" + include_examples "React Component", "div#HelloWorld-react-component-6" end context "render function that takes props" do - include_examples "React Component", "div#HelloWorldApp-react-component-7" + include_examples "React Component", "div#HelloWorld-react-component-7" end end end From e51769bb5045aa15fb874690454b68cbff8a68b7 Mon Sep 17 00:00:00 2001 From: Judah Meek Date: Mon, 27 Apr 2020 12:23:14 -0500 Subject: [PATCH 10/10] doc fixes --- CHANGELOG.md | 8 ++++---- CONTRIBUTING.md | 2 +- docs/basics/upgrading-react-on-rails.md | 14 +++++++------- docs/contributor-info/errors-with-hooks.md | 3 +-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f301c2a3f..f5829f1a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,12 +18,12 @@ Changes since last non-beta release. ### [12.0.0] #### BREAKING CHANGE -In order to solve the issues regarding React Hooks compatability, the number of parameters +In order to solve the issues regarding React Hooks compatibility, the number of parameters for functions is used to determine if you have a generator function that will get invoked to return a React component, or you are registering a functional React component. Alternately, you can set JavaScript property `renderFunction` on the function for which you want to return to be invoked to return the React component. In that case, you won't need to pass any unused params. -[PR 1268](https://github.com/shakacode/react_on_rails/pull/1268) by [justin808](https://github.com/justin808) +[PR 1268](https://github.com/shakacode/react_on_rails/pull/1268) by [justin808](https://github.com/justin808) See [docs/basics/upgrading-react-on-rails](./docs/basics/upgrading-react-on-rails#upgrading-to-v12) for details. @@ -59,9 +59,9 @@ by [ashgaliyev](https://github.com/ashgaliyev). #### Changed - Changed the default for: ```rb - config.raise_on_prerender_error = Rails.env.development? + config.raise_on_prerender_error = Rails.env.development? ``` - + Thus, developers will need to fix server rendering errors before continuing. [PR 1145](https://github.com/shakacode/react_on_rails/pull/1145) by [justin808](https://github.com/justin808). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 27f597d91..66b924a60 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -98,7 +98,7 @@ In addition to testing the Ruby parts out, you can also test the node package pa cd react_on_rails/ yarn yarn run build:watch -yalc pubish react-on-rails +yalc publish react-on-rails ``` Install the local package by using yalc, like this: diff --git a/docs/basics/upgrading-react-on-rails.md b/docs/basics/upgrading-react-on-rails.md index 28e6558e3..45750ccd7 100644 --- a/docs/basics/upgrading-react-on-rails.md +++ b/docs/basics/upgrading-react-on-rails.md @@ -6,19 +6,19 @@ If you would like help in migrating between React on Rails versions or help with We specialize in helping companies to quickly and efficiently move from versions before 9 to current. The older versions use the Rails asset pipeline to package client assets. The current and recommended way is to use Webpack 4 for asset preparation. You may also need help migrating from the `rails/webpacker`'s Webpack configuration to a better setup ready for Server Side Rendering. ## Upgrading to v13 -* Make sure that are are on a relatively more recent version of rails and webpacker. +* Make sure that you are on a relatively more recent version of rails and webpacker. * Updated API for ReactOnRails.register. -In order to solve the issues regarding React Hooks compatability, the number of parameters +In order to solve the issues regarding React Hooks compatibility, the number of parameters for functions is used to determine if you have a render function that will get invoked to return a React component, or you are registering a React component defined by a function. Registered Objects are of the following types: -##### Correct +##### Correct Either of these will work: 1. Take **2 params** and return **a React function or class component**. A function component is a function - that takes zero or one params and returns a React Element, like JSX. + that takes zero or one params and returns a React Element, like JSX. ```js export default (props, _railsContext) => () => ; ``` @@ -49,7 +49,7 @@ If you have a pure component, taking one or zero parameters, and you have an unn wrapper such that you're returning a function rather than a React Element, then: 1. You won't see anything render. -2. You will see this warning in development mode: `Warning: Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it.` +2. You will see this warning in development mode: `Warning: Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it.` --------- @@ -90,8 +90,8 @@ Reason for doing this: This enables your webpack bundles to bypass the Rails ass ##### ...while keeping your `client` directory * `.gitignore`: add `/public/webpack/*` * `Gemfile`: bump `react_on_rails` and add `webpacker` -* layout views: anything bundled by webpack will need to be requested by a `javascript_pack_tag` or `stylesheet_pack_tag`. - * Search your codebase for javascript_include_tag. Use the +* layout views: anything bundled by webpack will need to be requested by a `javascript_pack_tag` or `stylesheet_pack_tag`. + * Search your codebase for javascript_include_tag. Use the * `config/initializers/assets.rb`: we no longer need to modify `Rails.application.config.assets.paths` or append anything to `Rails.application.config.assets.precompile`. * `config/initializers/react_on_rails.rb`: * Delete `config.generated_assets_dir`. Webpacker's config now supplies this information diff --git a/docs/contributor-info/errors-with-hooks.md b/docs/contributor-info/errors-with-hooks.md index 75c420980..2d9b4e6d6 100644 --- a/docs/contributor-info/errors-with-hooks.md +++ b/docs/contributor-info/errors-with-hooks.md @@ -34,7 +34,7 @@ Make sure there is only one version of React installed! If you used yarn link, then you'll have two versions of React installed. -Instead use Yalc. +Instead use [Yalc](https://github.com/whitecolor/yalc). ``` cd @@ -43,4 +43,3 @@ yalc publish cd spec/dummy yalc link react-on-rails ``` -