-
-
Notifications
You must be signed in to change notification settings - Fork 631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Hydrate components immediately after downloading chunks #1656
base: abanoubghadban/pro362-add-support-for-RSC
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,8 @@ module Helper | |||||||||||||||
include ReactOnRails::Utils::Required | ||||||||||||||||
|
||||||||||||||||
COMPONENT_HTML_KEY = "componentHtml" | ||||||||||||||||
ADD_COMPONENT_TO_PENDING_HYDRATION_FUNCTION = "$ROR_PC" | ||||||||||||||||
ADD_STORE_TO_PENDING_HYDRATION_FUNCTION = "$ROR_PS" | ||||||||||||||||
|
||||||||||||||||
# react_component_name: can be a React function or class component or a "Render-Function". | ||||||||||||||||
# "Render-Functions" differ from a React function in that they take two parameters, the | ||||||||||||||||
|
@@ -362,13 +364,13 @@ def load_pack_for_generated_component(react_component_name, render_options) | |||||||||||||||
|
||||||||||||||||
ReactOnRails::PackerUtils.raise_nested_entries_disabled unless ReactOnRails::PackerUtils.nested_entries? | ||||||||||||||||
append_javascript_pack_tag("client-bundle") | ||||||||||||||||
# if Rails.env.development? | ||||||||||||||||
# is_component_pack_present = File.exist?(generated_components_pack_path(react_component_name)) | ||||||||||||||||
# raise_missing_autoloaded_bundle(react_component_name) unless is_component_pack_present | ||||||||||||||||
# end | ||||||||||||||||
# append_javascript_pack_tag("generated/#{react_component_name}", | ||||||||||||||||
# defer: ReactOnRails.configuration.defer_generated_component_packs) | ||||||||||||||||
# append_stylesheet_pack_tag("generated/#{react_component_name}") | ||||||||||||||||
if Rails.env.development? | ||||||||||||||||
is_component_pack_present = File.exist?(generated_components_pack_path(react_component_name)) | ||||||||||||||||
raise_missing_autoloaded_bundle(react_component_name) unless is_component_pack_present | ||||||||||||||||
end | ||||||||||||||||
append_javascript_pack_tag("generated/#{react_component_name}", | ||||||||||||||||
defer: ReactOnRails.configuration.defer_generated_component_packs) | ||||||||||||||||
append_stylesheet_pack_tag("generated/#{react_component_name}") | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity | ||||||||||||||||
|
@@ -401,6 +403,17 @@ def run_stream_inside_fiber | |||||||||||||||
rendering_fiber.resume | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def registered_stores | ||||||||||||||||
(@registered_stores || []) + (@registered_stores_defer_render || []) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we initialize the variables with |
||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def create_render_options(react_component_name, options) | ||||||||||||||||
# If no store dependencies are passed, default to all registered stores up till now | ||||||||||||||||
options[:store_dependencies] ||= registered_stores.map { |store| store[:store_name] } | ||||||||||||||||
ReactOnRails::ReactComponent::RenderOptions.new(react_component_name: react_component_name, | ||||||||||||||||
options: options) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def internal_stream_react_component(component_name, options = {}) | ||||||||||||||||
options = options.merge(stream?: true) | ||||||||||||||||
result = internal_react_component(component_name, options) | ||||||||||||||||
|
@@ -445,7 +458,7 @@ def build_react_component_result_for_server_rendered_string( | |||||||||||||||
|
||||||||||||||||
result_console_script = render_options.replay_console ? console_script : "" | ||||||||||||||||
result = compose_react_component_html_with_spec_and_console( | ||||||||||||||||
component_specification_tag, rendered_output, result_console_script | ||||||||||||||||
component_specification_tag, rendered_output, result_console_script, render_options.dom_id | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
prepend_render_rails_context(result) | ||||||||||||||||
|
@@ -511,12 +524,15 @@ def build_react_component_result_for_server_rendered_hash( | |||||||||||||||
) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def compose_react_component_html_with_spec_and_console(component_specification_tag, rendered_output, console_script) | ||||||||||||||||
def compose_react_component_html_with_spec_and_console(component_specification_tag, rendered_output, console_script, dom_id = nil) | ||||||||||||||||
add_component_to_pending_hydration_code = "window.#{ADD_COMPONENT_TO_PENDING_HYDRATION_FUNCTION}('#{dom_id}');" | ||||||||||||||||
hydrate_script = dom_id.present? ? content_tag(:script, add_component_to_pending_hydration_code.html_safe) : "" | ||||||||||||||||
Comment on lines
+528
to
+529
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's a bit clearer like this:
Suggested change
|
||||||||||||||||
# IMPORTANT: Ensure that we mark string as html_safe to avoid escaping. | ||||||||||||||||
html_content = <<~HTML | ||||||||||||||||
#{rendered_output} | ||||||||||||||||
#{component_specification_tag} | ||||||||||||||||
#{console_script} | ||||||||||||||||
#{hydrate_script} | ||||||||||||||||
HTML | ||||||||||||||||
html_content.strip.html_safe | ||||||||||||||||
end | ||||||||||||||||
|
@@ -528,10 +544,30 @@ def rails_context_if_not_already_rendered | |||||||||||||||
|
||||||||||||||||
@rendered_rails_context = true | ||||||||||||||||
|
||||||||||||||||
content_tag(:script, | ||||||||||||||||
json_safe_and_pretty(data).html_safe, | ||||||||||||||||
type: "application/json", | ||||||||||||||||
id: "js-react-on-rails-context") | ||||||||||||||||
rails_context_tag = content_tag(:script, | ||||||||||||||||
json_safe_and_pretty(data).html_safe, | ||||||||||||||||
type: "application/json", | ||||||||||||||||
id: "js-react-on-rails-context") | ||||||||||||||||
|
||||||||||||||||
pending_hydration_script = <<~JS.strip_heredoc | ||||||||||||||||
window.REACT_ON_RAILS_PENDING_COMPONENT_DOM_IDS = []; | ||||||||||||||||
window.REACT_ON_RAILS_PENDING_STORE_NAMES = []; | ||||||||||||||||
window.#{ADD_COMPONENT_TO_PENDING_HYDRATION_FUNCTION} = function(domId) { | ||||||||||||||||
window.REACT_ON_RAILS_PENDING_COMPONENT_DOM_IDS.push(domId); | ||||||||||||||||
if (window.ReactOnRails) { | ||||||||||||||||
window.ReactOnRails.renderOrHydrateLoadedComponents(); | ||||||||||||||||
} | ||||||||||||||||
}; | ||||||||||||||||
window.#{ADD_STORE_TO_PENDING_HYDRATION_FUNCTION} = function(storeName) { | ||||||||||||||||
window.REACT_ON_RAILS_PENDING_STORE_NAMES.push(storeName); | ||||||||||||||||
if (window.ReactOnRails) { | ||||||||||||||||
window.ReactOnRails.hydratePendingStores(); | ||||||||||||||||
} | ||||||||||||||||
}; | ||||||||||||||||
JS | ||||||||||||||||
rails_context_tag.concat( | ||||||||||||||||
content_tag(:script, pending_hydration_script.html_safe) | ||||||||||||||||
).html_safe | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
# prepend the rails_context if not yet applied | ||||||||||||||||
|
@@ -547,18 +583,20 @@ def internal_react_component(react_component_name, options = {}) | |||||||||||||||
# (re-hydrate the data). This enables react rendered on the client to see that the | ||||||||||||||||
# server has already rendered the HTML. | ||||||||||||||||
|
||||||||||||||||
render_options = ReactOnRails::ReactComponent::RenderOptions.new(react_component_name: react_component_name, | ||||||||||||||||
options: options) | ||||||||||||||||
render_options = create_render_options(react_component_name, options) | ||||||||||||||||
|
||||||||||||||||
# Setup the page_loaded_js, which is the same regardless of prerendering or not! | ||||||||||||||||
# The reason is that React is smart about not doing extra work if the server rendering did its job. | ||||||||||||||||
component_specification_tag = content_tag(:script, | ||||||||||||||||
json_safe_and_pretty(render_options.client_props).html_safe, | ||||||||||||||||
type: "application/json", | ||||||||||||||||
class: "js-react-on-rails-component", | ||||||||||||||||
id: "js-react-on-rails-component-#{render_options.dom_id}", | ||||||||||||||||
"data-component-name" => render_options.react_component_name, | ||||||||||||||||
"data-trace" => (render_options.trace ? true : nil), | ||||||||||||||||
"data-dom-id" => render_options.dom_id) | ||||||||||||||||
"data-dom-id" => render_options.dom_id, | ||||||||||||||||
"data-store-dependencies" => render_options.store_dependencies.to_json, | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
if render_options.force_load | ||||||||||||||||
component_specification_tag.concat( | ||||||||||||||||
|
@@ -580,12 +618,17 @@ def internal_react_component(react_component_name, options = {}) | |||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def render_redux_store_data(redux_store_data) | ||||||||||||||||
result = content_tag(:script, | ||||||||||||||||
store_hydration_data = content_tag(:script, | ||||||||||||||||
json_safe_and_pretty(redux_store_data[:props]).html_safe, | ||||||||||||||||
type: "application/json", | ||||||||||||||||
"data-js-react-on-rails-store" => redux_store_data[:store_name].html_safe) | ||||||||||||||||
hydration_code = "window.#{ADD_STORE_TO_PENDING_HYDRATION_FUNCTION}('#{redux_store_data[:store_name]}');" | ||||||||||||||||
store_hydration_script = content_tag(:script, hydration_code.html_safe) | ||||||||||||||||
|
||||||||||||||||
prepend_render_rails_context(result) | ||||||||||||||||
prepend_render_rails_context <<~HTML | ||||||||||||||||
#{store_hydration_data} | ||||||||||||||||
#{store_hydration_script} | ||||||||||||||||
HTML | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def props_string(props) | ||||||||||||||||
|
@@ -642,7 +685,7 @@ def server_rendered_react_component(render_options) | |||||||||||||||
js_code = ReactOnRails::ServerRenderingJsCode.server_rendering_component_js_code( | ||||||||||||||||
props_string: props_string(props).gsub("\u2028", '\u2028').gsub("\u2029", '\u2029'), | ||||||||||||||||
rails_context: rails_context(server_side: true).to_json, | ||||||||||||||||
redux_stores: initialize_redux_stores, | ||||||||||||||||
redux_stores: initialize_redux_stores(render_options), | ||||||||||||||||
react_component_name: react_component_name, | ||||||||||||||||
render_options: render_options | ||||||||||||||||
) | ||||||||||||||||
|
@@ -676,17 +719,18 @@ def server_rendered_react_component(render_options) | |||||||||||||||
result | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
def initialize_redux_stores | ||||||||||||||||
def initialize_redux_stores(render_options) | ||||||||||||||||
result = +<<-JS | ||||||||||||||||
ReactOnRails.clearHydratedStores(); | ||||||||||||||||
JS | ||||||||||||||||
|
||||||||||||||||
return result unless @registered_stores.present? || @registered_stores_defer_render.present? | ||||||||||||||||
store_dependencies = render_options.store_dependencies | ||||||||||||||||
return result unless store_dependencies.present? | ||||||||||||||||
|
||||||||||||||||
declarations = +"var reduxProps, store, storeGenerator;\n" | ||||||||||||||||
all_stores = (@registered_stores || []) + (@registered_stores_defer_render || []) | ||||||||||||||||
store_objects = registered_stores.select { |store| store_dependencies.include?(store[:store_name]) } | ||||||||||||||||
|
||||||||||||||||
result << all_stores.each_with_object(declarations) do |redux_store_data, memo| | ||||||||||||||||
result << store_objects.each_with_object(declarations) do |redux_store_data, memo| | ||||||||||||||||
store_name = redux_store_data[:store_name] | ||||||||||||||||
props = props_string(redux_store_data[:props]) | ||||||||||||||||
memo << <<-JS.strip_heredoc | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,9 +1,33 @@ | ||||||
import type { RegisteredComponent, ReactComponentOrRenderFunction, RenderFunction } from './types/index'; | ||||||
import React from 'react'; | ||||||
import type { RegisteredComponent, ReactComponentOrRenderFunction, RenderFunction, ReactComponent } from './types/index'; | ||||||
import isRenderFunction from './isRenderFunction'; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While changing this anyway, can simplify the import to |
||||||
|
||||||
const registeredComponents = new Map<string, RegisteredComponent>(); | ||||||
const registrationCallbacks = new Map<string, Array<(component: RegisteredComponent) => void>>(); | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better give a name to the callback type (here if it isn't used anywhere else, in |
||||||
export default { | ||||||
/** | ||||||
* Register a callback to be called when a specific component is registered | ||||||
* @param componentName Name of the component to watch for | ||||||
* @param callback Function called with the component details when registered | ||||||
*/ | ||||||
onComponentRegistered( | ||||||
componentName: string, | ||||||
callback: (component: RegisteredComponent) => void | ||||||
): void { | ||||||
// If component is already registered, schedule callback | ||||||
const existingComponent = registeredComponents.get(componentName); | ||||||
if (existingComponent) { | ||||||
setTimeout(() => callback(existingComponent), 0); | ||||||
return; | ||||||
} | ||||||
|
||||||
// Store callback for future registration | ||||||
const callbacks = registrationCallbacks.get(componentName) || []; | ||||||
callbacks.push(callback); | ||||||
registrationCallbacks.set(componentName, callbacks); | ||||||
}, | ||||||
|
||||||
/** | ||||||
* @param components { component1: component1, component2: component2, etc. } | ||||||
*/ | ||||||
|
@@ -21,15 +45,33 @@ export default { | |||||
const renderFunction = isRenderFunction(component); | ||||||
const isRenderer = renderFunction && (component as RenderFunction).length === 3; | ||||||
|
||||||
registeredComponents.set(name, { | ||||||
const registeredComponent = { | ||||||
name, | ||||||
component, | ||||||
renderFunction, | ||||||
isRenderer, | ||||||
}; | ||||||
registeredComponents.set(name, registeredComponent); | ||||||
|
||||||
const callbacks = registrationCallbacks.get(name) || []; | ||||||
callbacks.forEach(callback => { | ||||||
setTimeout(() => callback(registeredComponent), 0); | ||||||
}); | ||||||
registrationCallbacks.delete(name); | ||||||
}); | ||||||
}, | ||||||
|
||||||
registerServerComponent(...componentNames: string[]): void { | ||||||
// eslint-disable-next-line global-require, @typescript-eslint/no-var-requires | ||||||
const RSCClientRoot = require('./RSCClientRoot').default; | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For better types:
Suggested change
|
||||||
const componentsWrappedInRSCClientRoot = componentNames.reduce( | ||||||
(acc, name) => ({ ...acc, [name]: () => React.createElement(RSCClientRoot, { componentName: name }) }), | ||||||
{} | ||||||
); | ||||||
this.register(componentsWrappedInRSCClientRoot); | ||||||
}, | ||||||
|
||||||
/** | ||||||
* @param name | ||||||
* @returns { name, component, isRenderFunction, isRenderer } | ||||||
|
@@ -45,6 +87,12 @@ export default { | |||||
Registered component names include [ ${keys} ]. Maybe you forgot to register the component?`); | ||||||
}, | ||||||
|
||||||
async getOrWaitForComponent(name: string): Promise<RegisteredComponent> { | ||||||
return new Promise((resolve) => { | ||||||
this.onComponentRegistered(name, resolve); | ||||||
}); | ||||||
}, | ||||||
|
||||||
/** | ||||||
* Get a Map containing all registered components. Useful for debugging. | ||||||
* @returns Map where key is the component name and values are the | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can easily update
Configuration
so adding new options at least has to change only 2 places, not 4 :) Do you expect to need more changes like this soon?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I will need to add more configs like this soon. But it's useful if you want to implement a helper function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wouldn't be a helper function, just initialize to default values directly in the constructor and remove constructor parameters since we never end up using them (at least in RORP, need to double-check ROR before doing that).
I can wait until your PRs are merged to avoid conflicts anyway.