Adding a forced generator option

[Garethp]

What is this?

This allows someone to force ReactOnRails.register to treat what they registered as a generator function(s).

Why is this needed?

It seems like webpack somehow transforms generator functions in such a way that React no longer recognizes them. I'm not the only person, someone has already logged #1097 to ask for help. For that person, the solution was to use Webpacks useBuiltIns and then include babel-polyfil. Unfortunately, we're rendering our code using Googles V8 Engine. babel-polyfil currently causes a fatal error if the V8 engine tries to create a snapshot of babel-polyfil. Here's someone else reporting an issue with php-v8js. I've already logged a ticket with Google about this, but they might take time to fix and release. And then it might take a while for the release to propagate and so on.

On the other hand, a simple flag to force the registry to accept that it's being given a generator function would be much easier to implement and distribute, and would give another option other than to include babel-polyfil

---

This change is 

[coveralls]

Coverage remained the same at ?% when pulling 8924594 on Garethp:master into 0dbfb69 on shakacode:master.

[coveralls]

Coverage remained the same at ?% when pulling ef85237 on Garethp:master into 0dbfb69 on shakacode:master.

[justin808]

@Garethp I'm not totally convinced. Did you try this?

Also, if we were to do such a change, I'd like to see if the change could be only done here:

https://github.com/shakacode/react_on_rails/blob/master/node_package/src/generatorFunction.js

If you can provide me exact steps to repro this on MacOS, I'll take a look.

Additionally, I'd need tests, docs, etc. to merge.

Thanks for helping out.

[Garethp]

@justin808 Initially I did have babel-polyfil removed from my build, and that worked for us for the time, because we were just using generic React components and didn't need Generator Functions. Recently we needed to implement React.Helmet and that required we used Generator Functions. Since we compile our code using Webpack, the Generator Functions weren't actually registering AS generator functions (See #1097).

The problem is that the fix that is to tell webpack to use built ins. Telling webpack to use built ins without telling including babel-polyfil results in a lot of errors about runtimeGenerator (At least, that's the first issue we came across). The fix for that is to use babel-polyfil, hence the issue.

The goal of this PR is to make an alternative than to #1097 than to use webpack's useBuiltIns and babel-polyfil. I'll write up a quick repo for you in the next day or so, but it's essentially mirroring #1097, except using babel-polyfil isn't an option for us.

To be honest I'm not sure how I'd fix it solely in generatorFunction. Still, once I get a repo for you to pull and try, maybe you can find a better way of fixing this. Personally, I thought that this solution might be more future-proof as it allows you to override the automatic detection of whether something's a generator function or a react component if you the developer know's that it's a generator function.

[justin808]

We started off with having an explicit flag for this. We spent a ton of time trying to make the error messages super clear when you mess up. And a super long time documenting this. Ideally, the idea of the "generator function" should be renamed to "HOC" Higher Order Function. So I'm really open to getting this properly handled. And we need to be careful about adding an API and then wanting to change it.

[justin808]

Thanks @Garethp for your efforts!

Please provide:

• tests
• compelling examples I can run that confirm this is necessary.

Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Garethp)

---

package.json, line 56 at r2 (raw file):

    "test": "babel-tape-runner -r node_package/tests/helpers/test_helper.js node_package/tests/*.js | tap-spec",
    "clean": "rm -rf node_package/lib",
    "prepare": "npm run babel",

why is this changing?

[justin808]

Any update on this? Anybody else seeing this issue?

[Garethp]

Sorry for the delay. I haven't found the time to package it up in to a simple and self contained example. Here's something you can clone and reproduce. The docker container will take care of the php stuff, the js just needs to be built outside of the docker container.

If you run the docker as is, you should see a fatal error with a stack track. I've already reported the stack error to Google, as mentioned in the description for this ticket. You can change the entry point of the Dockerfile to the other .php file I included to show that the issue isn't caused by the library I'm using in PHP to render this, but rather an issue with the V8 engine itself.

Now, the cause for this specific stack trace is babel-polyfill. You can remove the babel-polyfill from the webpack entry, but then if you try to build and run the js npm run build && node ../server.js you should see an error along the lines of:

webpack:///./Home.js?:35
            var _ref = _asyncToGenerator( /*#__PURE__*/regeneratorRuntime.mark(function _callee() {
                                                       ^

ReferenceError: regeneratorRuntime is not defined
    at eval (webpack:///./Home.js?:35:56)
    at eval (webpack:///./Home.js?:52:10)
    at eval (webpack:///./Home.js?:65:2)
    at Object../Home.js (/home/gareth/development/react-on-rails-bug/server.js:97:1)
    at __webpack_require__ (/home/gareth/development/react-on-rails-bug/server.js:20:30)
    at eval (webpack:///./index.js?:11:13)
    at Object../index.js (/home/gareth/development/react-on-rails-bug/server.js:109:1)
    at __webpack_require__ (/home/gareth/development/react-on-rails-bug/server.js:20:30)
    at eval (webpack:///multi_./index.js?:1:18)
    at Object.0 (/home/gareth/development/react-on-rails-bug/server.js:1603:1)

This is 'caused by

    async doesThing() {

    }

I put it in there as a way to show this error, because we actually have quite a few async functions in our code base. The async causing the regeneratorRuntime error is because of babels useBuiltIns. However, if we remove the useBuiltIns and rebuild, run the docker container again and you should see

Attaching to react-on-rails-bug_app_1
app_1  | array(1) {
app_1  |   ["componentHtml"]=>
app_1  |   string(3243) "<div id="sfreact-reactRenderer5bacbdde352db9.98235599"><pre data-reactroot="">Exception in rendering!
app_1  | 
app_1  | Message: 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.
app_1  |     in wrapper
app_1  | 
app_1  | Invariant Violation: 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.
app_1  |     in wrapper
app_1  |     at invariant (webpack:///./node_modules/react/cjs/react.development.js?:127:15)
app_1  |     at traverseAllChildrenImpl (webpack:///./node_modules/react/cjs/react.development.js?:1123:7)
app_1  |     at traverseAllChildren (webpack:///./node_modules/react/cjs/react.development.js?:1151:10)
app_1  |     at mapIntoWithKeyPrefixInternal (webpack:///./node_modules/react/cjs/react.development.js?:1229:3)
app_1  |     at toArray (webpack:///./node_modules/react/cjs/react.development.js?:1278:3)
app_1  |     at ReactDOMServerRenderer.render (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:2720:28)
app_1  |     at ReactDOMServerRenderer.read (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:2679:23)
app_1  |     at Object.renderToString (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:3070:25)
app_1  |     at serverRenderReactComponent (webpack:///./node_modules/react-on-rails/node_package/lib/serverRenderReactComponent.js?:85:37)
app_1  |     at Object.serverRenderReactComponent (webpack:///./node_modules/react-on-rails/node_package/lib/ReactOnRails.js?:244:53)</pre>
app_1  | <script id="consoleReplayLog">
app_1  | console.error.apply(console, ["[SERVER] Exception in rendering!"]);
app_1  | console.error.apply(console, ["[SERVER] message: 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 wrapper"]);
app_1  | console.error.apply(console, ["[SERVER] stack: Invariant Violation: 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 wrapper\n    at invariant (webpack:///./node_modules/react/cjs/react.development.js?:127:15)\n    at traverseAllChildrenImpl (webpack:///./node_modules/react/cjs/react.development.js?:1123:7)\n    at traverseAllChildren (webpack:///./node_modules/react/cjs/react.development.js?:1151:10)\n    at mapIntoWithKeyPrefixInternal (webpack:///./node_modules/react/cjs/react.development.js?:1229:3)\n    at toArray (webpack:///./node_modules/react/cjs/react.development.js?:1278:3)\n    at ReactDOMServerRenderer.render (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:2720:28)\n    at ReactDOMServerRenderer.read (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:2679:23)\n    at Object.renderToString (webpack:///./node_modules/react-dom/cjs/react-dom-server.browser.development.js?:3070:25)\n    at serverRenderReactComponent (webpack:///./node_modules/react-on-rails/node_package/lib/serverRenderReactComponent.js?:85:37)\n    at Object.serverRenderReactComponent (webpack:///./node_modules/react-on-rails/node_package/lib/ReactOnRails.js?:244:53)"]);
app_1  | </script></div>"
app_1  | }
react-on-rails-bug_app_1 exited with code 0

Because the generator function isn't being detected as a generator function. Hence my fix.

As for the changing of the build, that was for my workplace. Since you want to tackle this bug in a different method than how I did it, I ended up changing the master branch of my fork to suit our build environment, assuming that my PR would be rejected for another approach.

Is there any more information you need?

[justin808]

@Garethp can you update your repo to the current version of React on Rails?

[justin808]

@Garethp

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Garethp)

---

node_package/src/ComponentRegistry.js, line 22 at r2 (raw file):

      }

      const isGeneratorFunction = generatorFunction(component);

@Garethp have you exhausted all possibilities to fix the function generatorFunction(component)?

If we go with your fix, we need to document this.

[Garethp]

Hi

I didn't explore all possibilities because to be quite honest I had already spent quite a bit of time investigating this issue and reporting it to the various vendors, and didn't want to spend further time in an area had evidently had quite a bit of research put in to it by you. I had planned to come back to it later on to investigate further, but that time never came.

At the moment, I'm not longer working for the company that I implemented this for, so I probably won't be following this bug much further unless I end up using ReactOnRails in my next position. I hope you can end up finding a more elegant solution though.

[justin808]

@Garethp I'd definitely merge something if others also report this issue.