WIP: Enable registered components to return promises for server rendering by React on Rails Pro Node Renderer #1380

Judahmeek · 2021-08-07T18:38:09Z

This change is

Judahmeek · 2021-08-10T10:42:39Z

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb

@@ -150,6 +150,10 @@ def create_js_context
        end

        def execjs_timer_polyfills
+          if ReactOnRails::Utils.react_on_rails_pro? && ReactOnRailsPro.configuration.server_renderer == "NodeRenderer"
+            return ""
+          end


Removes all execjs_timer_polyfills if node rendering.

Judahmeek · 2021-08-10T10:48:31Z

node_package/src/types/index.ts

@@ -105,7 +106,7 @@ export interface RenderingError {
 }

 export interface RenderResult {
-  html: string;
+  html: string | null;


Because await statements have a return type of optional null

Judahmeek · 2021-08-10T10:51:55Z

node_package/src/serverRenderReactComponent.ts

+  if(returnPromise) {
+    const resolveRenderResult = async () => {
+      const promiseResult = {
+        html: await renderResult,


The entire promiseResult gets returned by this async function as a promise, but an await statement doesn't seem to resolve internal promises, so I resolve the promise we get from the react component here.

Judahmeek · 2021-08-10T10:52:47Z

node_package/src/serverRenderReactComponent.ts

+      message: renderingError.message,
+      stack: renderingError.stack,
+    };
+  }


I put this logic in a function so this logic can be used inside the async function as well as normally.

Judahmeek · 2021-08-10T10:54:08Z

node_package/src/createReactOutput.ts

+      // We just return at this point, because calling function knows how to handle this case and
+      // we can't call React.createElement with this type of Object.
+      return (renderFunctionResult as Promise<string>);
+    }


Same logic as before, just with different types to make TypeScript happy

justin808 · 2021-08-10T16:10:15Z

node_package/src/clientStartup.ts

@@ -8,7 +8,7 @@ import type {
 } from './types/index';

 import createReactOutput from './createReactOutput';
-import isServerRenderResult from './isServerRenderResult';
+import {isServerRenderHash} from './isServerRenderResult';


linting should put in some spaces

Let’s do linting right after this merges. I’m surprised it’s not already there!

justin808

Reviewed 7 of 7 files at r1, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Judahmeek)

-- commits, line 12 at r3 ([raw file](https://github.com/shakacode/react_on_rails/blob/a754f557e88aea05ca8386af51c76779f4a62d2b/-- commits#L12)):
What is this file?

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 155 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

Removes all execjs_timer_polyfills if node rendering.

This needs to be clearly documented in the changelog.

And major version bump? Not sure, since it requires React on Rails Pro for htis.

node_package/src/createReactOutput.ts, line 57 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

Same logic as before, just with different types to make TypeScript happy

OK

node_package/src/serverRenderReactComponent.ts, line 93 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

I put this logic in a function so this logic can be used inside the async function as well as normally.

OK

node_package/src/serverRenderReactComponent.ts, line 98 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

The entire promiseResult gets returned by this async function as a promise, but an await statement doesn't seem to resolve internal promises, so I resolve the promise we get from the react component here.

strange, but OK

node_package/src/serverRenderReactComponent.ts, line 13 at r3 (raw file):

export default function serverRenderReactComponent(options: RenderParams): null | string | Promise<RenderResult> {
  const { name, domNodeId, trace, props, railsContext, returnPromise, throwJsErrors } = options;

Who sets "returnPromise" option?

node_package/src/types/index.ts, line 109 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

Because await statements have a return type of optional null

OK

Judahmeek

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @justin808)

-- commits, line 12 at r3 ([raw file](https://github.com/shakacode/react_on_rails/blob/a754f557e88aea05ca8386af51c76779f4a62d2b/-- commits#L12)):

Previously, justin808 (Justin Gordon) wrote…

What is this file?

It's a new Reviewable feature. They should change their code so that they aren't reusing the same component as they use for their files so people don't get confused like you did.

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 155 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

This needs to be clearly documented in the changelog.

And major version bump? Not sure, since it requires React on Rails Pro for htis.

A major version bump wouldn't hurt.

node_package/src/clientStartup.ts, line 11 at r3 (raw file):

Previously, justin808 (Justin Gordon) wrote…

linting should put in some spaces

Do you want me to add that linting change in this PR or another PR?

node_package/src/serverRenderReactComponent.ts, line 13 at r3 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Who sets "returnPromise" option?

ReactOnRailsPro::ServerRenderingJsCode.render will once https://github.com/shakacode/react_on_rails_pro/pull/210 is merged

justin808

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

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 155 at r1 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

A major version bump wouldn't hurt.

Please add the changelog entry.

I do think we'd be better we could toggle this, then it's a minor feature bump.

justin808

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

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 155 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Please add the changelog entry.

I do think we'd be better we could toggle this, then it's a minor feature bump.

Actually, not a major version bump for React on Rails, but one for React on Rails Pro, but no, let's make this configurable.

justin808

Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Judahmeek)

CHANGELOG.md, line 22 at r4 (raw file):

#### Added

- Ability to skip stubbing of setTimeout, setInterval, & clearTimeout polyfills when using properly configured React on Rails Pro. Also, ability to have render functions return a promise to be awaited by React on Rails Pro Node Renderer. [PR 1380](https://github.com/shakacode/react_on_rails/pull/1380) by [judahmeek](https://github.com/judahmeek)

Needs summary here on the configuration change.

Needs changes to doc files.

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 153 at r4 (raw file):
not operator?

ReactOnRailsPro.configuration.execjs_polyfills

 if ReactOnRails::Utils.react_on_rails_pro? && !ReactOnRailsPro.configuration.execjs_polyfills

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

export default function serverRenderReactComponent(options: RenderParams): null | string | Promise<RenderResult> {
  const { name, domNodeId, trace, props, railsContext, returnPromise, throwJsErrors } = options;

can we find a more descriptive name than returnPromise?

maybe rendererReturnsPromises

I'm not sure where this comes from.

if we have to call the rendering helper with knowledge of the return type, could we use returnsPromise

also, could this be just inferred? so we don't need an option on the helper?

node_package/src/serverRenderReactComponent.ts, line 35 at r4 (raw file):

    });

    if (isServerRenderHash(reactElementOrRouterResult)) {

Should we consider checking if a promise first?

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

        renderResult = (reactElementOrRouterResult as { renderedHtml: string }).renderedHtml;
      }
    } else if (isPromise(reactElementOrRouterResult)) {

Maybe the check for this error condition can be encapsulated in a separate function and checked a bit earlier.

this giant if is hard to follow.

node_package/src/serverRenderReactComponent.ts, line 60 at r4 (raw file):

      }
    } else if (isPromise(reactElementOrRouterResult)) {
      if (!returnPromise) {

maybe rendererReturnsPromises

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

    const resolveRenderResult = async () => {
      const promiseResult = {
        html: await renderResult,

let's have this await be its own statement with its own catch block with error handling

Judahmeek

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @justin808)

CHANGELOG.md, line 22 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Needs summary here on the configuration change.

Needs changes to doc files.

I document the configuration change in the React on Rails Pro PR.

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 155 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Actually, not a major version bump for React on Rails, but one for React on Rails Pro, but no, let's make this configurable.

Done

lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb, line 153 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

not operator?

ReactOnRailsPro.configuration.execjs_polyfills

 if ReactOnRails::Utils.react_on_rails_pro? && !ReactOnRailsPro.configuration.execjs_polyfills

Can do

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

can we find a more descriptive name than returnPromise?

maybe rendererReturnsPromises

I'm not sure where this comes from.

if we have to call the rendering helper with knowledge of the return type, could we use returnsPromise

also, could this be just inferred? so we don't need an option on the helper?

The parameter comes from React on Rails Pro server_render_js_code.

I called the parameter returnPromise because the parameter is telling the serverRenderReactComponent to return a promise.

We could infer this by having `serverRenderReactComponent return a promise whenever the NodeRenderer is being used instead.

node_package/src/serverRenderReactComponent.ts, line 35 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Should we consider checking if a promise first?

See below

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Maybe the check for this error condition can be encapsulated in a separate function and checked a bit earlier.

this giant if is hard to follow.

I'll see what I can do to make all this conditional logic easier to read & comprehend.

I think I'll wrap the inner logic inside functions, so you'll just see:

    if (isServerRenderHash(reactElementOrRouterResult)) {
      renderResult = processServerHash(reactElementOrRouterResult)
    } else if (isPromise(reactElementOrRouterResult)) {
      renderResult = processPromise(reactElementOrRouterResult);
    } else {
      renderResult = renderReactElement(reactElementOrRouterResult)
    }

node_package/src/serverRenderReactComponent.ts, line 60 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

maybe rendererReturnsPromises

No, the parameter is telling the serverRenderReactComponent function to return a promise.

If this function isn't returning a promise, however, then it's going to use JSON.stringify on whatever result it got from the render function, so I'm logging an error here.

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

let's have this await be its own statement with its own catch block with error handling

How should the error be handled?

Should I use the same logic from above?

} catch (e) {
    if (throwJsErrors) {
      throw e;
    }

    hasErrors = true;
    renderResult = handleError({
      e,
      name,
      serverSide: true,
    });
    renderingError = e;
  }

justin808

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Judahmeek and @justin808)

CHANGELOG.md, line 22 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

I document the configuration change in the React on Rails Pro PR.

We can mention that here in the CHANGELOG and indicate that it's not relevant to the non-pro version.

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

The parameter comes from React on Rails Pro server_render_js_code.

I called the parameter returnPromise because the parameter is telling the serverRenderReactComponent to return a promise.

We could infer this by having `serverRenderReactComponent return a promise whenever the NodeRenderer is being used instead.

I think that's a much better solution. Many APIs work like this. AFAIK, there's no performance hit given that the majority of the work is doing the SSR.

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

I'll see what I can do to make all this conditional logic easier to read & comprehend.

I think I'll wrap the inner logic inside functions, so you'll just see:

    if (isServerRenderHash(reactElementOrRouterResult)) {
      renderResult = processServerHash(reactElementOrRouterResult)
    } else if (isPromise(reactElementOrRouterResult)) {
      renderResult = processPromise(reactElementOrRouterResult);
    } else {
      renderResult = renderReactElement(reactElementOrRouterResult)
    }

This might become simpler if we always return a promise, right?

And yes, much better.

I think the name reactElementOrRouterResult can be shorted to reactRenderingResult as the current name does not reflect it might be a promise.

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

How should the error be handled?

Should I use the same logic from above?

} catch (e) {
    if (throwJsErrors) {
      throw e;
    }

    hasErrors = true;
    renderResult = handleError({
      e,
      name,
      serverSide: true,
    });
    renderingError = e;
  }

Yes, that should work.

We might need to rethrow. Check the logic of the other blocks.

justin808

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Judahmeek and @justin808)

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

export default function serverRenderReactComponent(options: RenderParams): null | string | Promise<RenderResult> {
  const { name, domNodeId, trace, props, railsContext, returnPromise, throwJsErrors } = options;

How about this for the new name:

renderingReturnsPromises

And it's set on the React on Rails Pro side.

node_package/src/serverRenderReactComponent.ts, line 61 at r4 (raw file):

    } else if (isPromise(reactElementOrRouterResult)) {
      if (!returnPromise) {
        console.error(`Your render function returned a Promise, which is only supported by a node renderer, not ExceJS.`)

This logic implies that returnPromise really means renderedWithNodeRenderer

I think changing the name to renderingReturnsPromises and that's optionally set from ReactOnRailsPro will work.

Judahmeek

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Judahmeek and @justin808)

CHANGELOG.md, line 22 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

We can mention that here in the CHANGELOG and indicate that it's not relevant to the non-pro version.

Something like this perhaps?

Ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional on NEW execjs_polyfills React on Rails Pro configuration option.

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

This might become simpler if we always return a promise, right?

No. If using ExecJS, the result from serverRenderReactComponent turns into a string & promises don't serialize.

node_package/src/serverRenderReactComponent.ts, line 61 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

This logic implies that returnPromise really means renderedWithNodeRenderer

I think changing the name to renderingReturnsPromises and that's optionally set from ReactOnRailsPro will work.

Yes, that implication is correct. rendererAcceptsPromises is another way you could phrase it.

Do you want an optional parameter from ReactOnRailsPro or do you want to check if ReactOnRailsPro.configuration.server_renderer == "NodeRenderer"?

justin808

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Judahmeek and @justin808)

node_package/src/serverRenderReactComponent.ts, line 61 at r4 (raw file):

Ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional on NEW execjs_polyfills React on Rails Pro configuration option.

Let's use an additional parameter. I don't want to change the current behavior for all current installations.

ReactOnRailsPro.renderingReturnsPromises = true

Default is false.

justin808

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Judahmeek and @justin808)

CHANGELOG.md, line 22 at r4 (raw file):

NEW execjs_polyfills React on Rails Pro configuration option
I don't understand the execjs_polyfills name.

Maybe it should be include_execjs_polyfills

Ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional by setting ReactOnRailsPro.config.include_execjs_polyfills = false in the React on Rails Pro configuration file.

Judahmeek

Reviewable status: 7 of 12 files reviewed, 7 unresolved discussions (waiting on @justin808)

CHANGELOG.md, line 22 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

NEW execjs_polyfills React on Rails Pro configuration option
I don't understand the execjs_polyfills name.

Maybe it should be include_execjs_polyfills

Ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional by setting ReactOnRailsPro.config.include_execjs_polyfills = false in the React on Rails Pro configuration file.

Done.

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I think that's a much better solution. Many APIs work like this. AFAIK, there's no performance hit given that the majority of the work is doing the SSR.

Done.

node_package/src/serverRenderReactComponent.ts, line 13 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

How about this for the new name:
renderingReturnsPromises
And it's set on the React on Rails Pro side.

Done

node_package/src/serverRenderReactComponent.ts, line 61 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional on NEW execjs_polyfills React on Rails Pro configuration option.

Let's use an additional parameter. I don't want to change the current behavior for all current installations.
ReactOnRailsPro.renderingReturnsPromises = true
Default is false.

Done.

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Yes, that should work.

We might need to rethrow. Check the logic of the other blocks.

Done.

Btw, the current logic here just awaits & then submits the resolved value, which doesn't match the logic we use for normal serverRendering

This is why I made these changes for my original proof of concept.

We definitely need a more robust solution.

Should we await the renderResult & then run the conditional logic on the resolved value just in case the client is using both react-router & a graphql library (in which case we might have a promise resolve a redirect or routeError)?

gastonmorixe · 2021-08-26T19:40:46Z

Awesome work guys, keep me posted when this is finished so I can give it a try. @Judahmeek @justin808

justin808

@Judahmeek Getting close

We need to pair on the promises block of code.

Reviewed 3 of 5 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Judahmeek and @justin808)

node_package/babel.config.js, line 2 at r6 (raw file):

module.exports = {
  presets: [['@babel/preset-env', { targets: { node: 'current' } }]],

@Judahmeek, what motivated this one?

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

Done.

Btw, the current logic here just awaits & then submits the resolved value, which doesn't match the logic we use for normal serverRendering

This is why I made these changes for my original proof of concept.

We definitely need a more robust solution.

Should we await the renderResult & then run the conditional logic on the resolved value just in case the client is using both react-router & a graphql library (in which case we might have a promise resolve a redirect or routeError)?

Can the server do anything with routeError or redirects? We'll need to pair on these changes.

Judahmeek

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @justin808)

node_package/babel.config.js, line 2 at r6 (raw file):

Previously, justin808 (Justin Gordon) wrote…

@Judahmeek, what motivated this one?

await/promise support for jest testing

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Can the server do anything with routeError or redirects? We'll need to pair on these changes.

The NodeRenderer or the Rails server?

justin808

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Judahmeek)

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

This might become simpler if we always return a promise, right?

No. If using ExecJS, the result from serverRenderReactComponent turns into a string & promises don't serialize.

Looks like you did this, even though you wrote "No".

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

The NodeRenderer or the Rails server?

both

Judahmeek

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @justin808)

node_package/src/serverRenderReactComponent.ts, line 59 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Looks like you did this, even though you wrote "No".

I'm not sure why you say that.

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

both

Theoretically, the rails server could conditionally make another request to the NodeRenderer if the NodeRenderer returns a Redirect notice, in which case the logic would be something like:

Rails makes request to NodeRenderer
NodeRenderer returns the redirect object
Rails checks the NodeRenderResult to see if it is a redirect
If a redirect, Rails sends a new render request with the corrected location & props

My naive assumption is that there could only possibly be a single redirect per HTML request, but we could always throw a RenderError if there happens to be a second redirect for whatever reason.

justin808

All files are reviewed. LGTM.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @justin808)

justin808

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

node_package/src/serverRenderReactComponent.ts, line 101 at r4 (raw file):

Previously, Judahmeek (Judah Meek) wrote…

Theoretically, the rails server could conditionally make another request to the NodeRenderer if the NodeRenderer returns a Redirect notice, in which case the logic would be something like:

Rails makes request to NodeRenderer

NodeRenderer returns the redirect object

Rails checks the NodeRenderResult to see if it is a redirect

If a redirect, Rails sends a new render request with the corrected location & props

My naive assumption is that there could only possibly be a single redirect per HTML request, but we could always throw a RenderError if there happens to be a second redirect for whatever reason.

I agree with your assessment above, although the renderer does not have to be the node renderer.

I really have no idea about this use case. I suspect that the client can handle the redirect and return the correct path. While maybe not optimal, I don't see a reason to worry about this right now.

Added ability to stop stubbing of setTimeout, setInterval, & clearTimeout conditional by setting `ReactOnRailsPro.config.include_execjs_polyfills = false` in the React on Rails Pro configuration file. Also, added the ability to have render functions return a promise to be awaited by React on Rails Pro Node Renderer. [PR 1380](shakacode/react_on_rails#1380) by [judahmeek](https://github.com/judahmeek)

Judahmeek commented Aug 10, 2021

View reviewed changes

Judahmeek force-pushed the judahmeek/ssr-promise branch from ffe12e3 to a754f55 Compare August 10, 2021 10:56

justin808 reviewed Aug 11, 2021

View reviewed changes

justin808 requested changes Aug 11, 2021

View reviewed changes

Judahmeek commented Aug 17, 2021

View reviewed changes

Judahmeek requested a review from justin808 August 17, 2021 04:40

justin808 approved these changes Aug 17, 2021

View reviewed changes

Judahmeek force-pushed the judahmeek/ssr-promise branch from a754f55 to 027d135 Compare August 24, 2021 07:20

justin808 requested changes Aug 24, 2021

View reviewed changes

Judahmeek commented Aug 24, 2021

View reviewed changes

justin808 requested changes Aug 24, 2021

View reviewed changes

Judahmeek commented Aug 24, 2021

View reviewed changes

justin808 requested changes Aug 24, 2021

View reviewed changes

Judahmeek commented Aug 25, 2021

View reviewed changes

Judahmeek requested a review from justin808 August 26, 2021 07:11

justin808 requested changes Aug 28, 2021

View reviewed changes

Judahmeek commented Aug 28, 2021

View reviewed changes

justin808 requested changes Aug 29, 2021

View reviewed changes

Judahmeek commented Sep 20, 2021

View reviewed changes

Judahmeek requested a review from justin808 September 20, 2021 04:55

justin808 reviewed Sep 20, 2021

View reviewed changes

justin808 requested changes Sep 21, 2021

View reviewed changes

Judahmeek added 8 commits September 20, 2021 20:45

conditionally return promise rather than string

6825e59

modify server_exec_js js_code to conditionally return a promise

efb18f1

dont stub settimeout when using node renderer

6354b96

enable node rendered components to return promises

fa51512

linting

2c39f99

per review

28092fa

per review, add jest test

66a4480

prettier linting

a982847

justin808 force-pushed the judahmeek/ssr-promise branch from 97d749d to a982847 Compare September 21, 2021 06:49

justin808 merged commit 565957f into master Sep 21, 2021

justin808 deleted the judahmeek/ssr-promise branch September 21, 2021 06:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Enable registered components to return promises for server rendering by React on Rails Pro Node Renderer #1380

WIP: Enable registered components to return promises for server rendering by React on Rails Pro Node Renderer #1380

Judahmeek commented Aug 7, 2021 •

edited by justin808

Loading

Judahmeek Aug 10, 2021

Judahmeek Aug 10, 2021

Judahmeek Aug 10, 2021

Judahmeek Aug 10, 2021

Judahmeek Aug 10, 2021

justin808 Aug 10, 2021

justin808 Aug 17, 2021

justin808 left a comment

Judahmeek left a comment

justin808 left a comment

justin808 left a comment

justin808 left a comment

Judahmeek left a comment

justin808 left a comment

justin808 left a comment

Judahmeek left a comment

justin808 left a comment

justin808 left a comment

Judahmeek left a comment

gastonmorixe commented Aug 26, 2021

justin808 left a comment

Judahmeek left a comment

justin808 left a comment

Judahmeek left a comment

justin808 left a comment

justin808 left a comment

WIP: Enable registered components to return promises for server rendering by React on Rails Pro Node Renderer #1380

WIP: Enable registered components to return promises for server rendering by React on Rails Pro Node Renderer #1380

Conversation

Judahmeek commented Aug 7, 2021 • edited by justin808 Loading

Judahmeek Aug 10, 2021

Choose a reason for hiding this comment

Judahmeek Aug 10, 2021

Choose a reason for hiding this comment

Judahmeek Aug 10, 2021

Choose a reason for hiding this comment

Judahmeek Aug 10, 2021

Choose a reason for hiding this comment

Judahmeek Aug 10, 2021

Choose a reason for hiding this comment

justin808 Aug 10, 2021

Choose a reason for hiding this comment

justin808 Aug 17, 2021

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

gastonmorixe commented Aug 26, 2021

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

justin808 left a comment

Choose a reason for hiding this comment

Judahmeek commented Aug 7, 2021 •

edited by justin808

Loading