Skip to content
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

Adds benchmark option; Adds more tests; #101

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/react/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Railtie < ::Rails::Railtie
config.react.addons = false
# Server-side rendering
config.react.max_renderers = 10
config.react.benchmark = false
config.react.timeout = 20 #seconds
config.react.react_js = lambda {File.read(::Rails.application.assets.resolve('react.js'))}
config.react.component_filenames = ['components.js']
Expand Down Expand Up @@ -72,7 +73,7 @@ class Railtie < ::Rails::Railtie
do_setup = lambda do
cfg = app.config.react
React::Renderer.setup!( cfg.react_js, cfg.components_js,
{:size => cfg.size, :timeout => cfg.timeout})
{:size => cfg.size, :timeout => cfg.timeout, :benchmark => cfg.benchmark})
end

do_setup.call
Expand Down
17 changes: 14 additions & 3 deletions lib/react/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ def initialize(component_name, props, js_message)
cattr_accessor :pool

def self.setup!(react_js, components_js, args={})
args.assert_valid_keys(:size, :timeout)
args.assert_valid_keys(:size, :timeout, :benchmark)
@@react_js = react_js
@@components_js = components_js
@@pool.shutdown{} if @@pool
reset_combined_js!
@@render_method = args[:benchmark] ? :render_with_benchmark : :render
@@pool = ConnectionPool.new(:size => args[:size]||10, :timeout => args[:timeout]||20) { self.new }
end

def self.render(component, args={})
@@pool.with do |renderer|
renderer.render(component, args)
renderer.send(@@render_method, component, args)
end
end

Expand Down Expand Up @@ -74,8 +75,18 @@ def render(component, args={})
}()
JS
context.eval(jscode).html_safe
rescue ExecJS::ProgramError => e
rescue ::ExecJS::ProgramError => e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be just be requiring execjs in here?

raise PrerenderError.new(component, react_props, e)
end

def render_with_benchmark(component, args={})
result = ''
time = Benchmark.realtime do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do Benchmark.ms instead, which is what rails uses under the hood, so you don't need to do time*1000

result = self.render(component, args)
end
::Rails.logger.info "[React::Renderer#render] Rendered #{component} with #{args} (#{(time*1000).round(1)}ms)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a test for this too?

result
end

end
end
26 changes: 26 additions & 0 deletions test/react_renderer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,32 @@ class ReactRendererTest < ActiveSupport::TestCase
assert_match /data-react-checksum/, result
end

test 'prerender errors are thrown with non-json-encoded string' do
not_json_string = %w{todo1 todo2 todo3}.to_s

err = assert_raises React::Renderer::PrerenderError do
React::Renderer.render "TodoList", not_json_string
end
expected_message = 'Encountered error "Error: Invariant Violation: Tried to merge an object, instead got todo1,todo2,todo3." when prerendering TodoList with ["todo1", "todo2", "todo3"]'
assert_equal expected_message, err.message
end

test 'Server rendering with a simple Ruby Hash' do
a_hash = {todos: %w{todo1 todo2 todo3}}

result = React::Renderer.render "TodoList", a_hash
assert_match /todo1.*todo2.*todo3/, result
assert_match /data-react-checksum/, result
end

test 'Server rendering with a complex Ruby Hash' do
a_hash_with_nest = {data: {some: {thing: [0,1,2]}}, todos: [:some,:nested,:data]}

result = React::Renderer.render "TodoList", a_hash_with_nest
assert_match /some.*nested.*data/, result
assert_match /data-react-checksum/, result
end

test 'Rendering does not throw an exception when console log api is used' do
%W(error info log warn).each do |fn|
assert_nothing_raised(ExecJS::ProgramError) do
Expand Down
34 changes: 33 additions & 1 deletion test/view_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,38 @@ class ViewHelperTest < ActionDispatch::IntegrationTest
assert html.include?('data-foo="1"')
end

test 'react_component prerender:true and a simple Ruby Hash does not raise' do
a_hash = {todos: %w{todo1 todo2 todo3}}

assert_nothing_raised(StandardError) do
@helper.react_component('TodoList', a_hash, {prerender:true, tag: :span})
end
end

test 'react_component prerender:true and a complex Ruby Hash does not raise' do
a_hash = {data: {some: {thing: [0,1,2]}}, todos: [:some,:nested,:data]}

assert_nothing_raised(StandardError) do
@helper.react_component('TodoList', a_hash, {prerender:true, tag: :span})
end
end

test 'react_component prerender:true and a simple Ruby Hash' do
a_hash = {todos: %w{todo1 todo2 todo3}}

html = @helper.react_component('TodoList', a_hash, {prerender:true, tag: :span})
assert html.match(/<span\s.*><\/span>/)
assert html.match(/<li\sdata-reactid=\".*\">todo1<\/li>/)
end

test 'react_component prerender:true and a complex Ruby Hash' do
a_hash = {data: {some: {thing: [0,1,2]}}, todos: [:some,:nested,:data]}

html = @helper.react_component('TodoList', a_hash, {prerender:true, tag: :span})
assert html.match(/<span\s.*><\/span>/)
assert html.match(/<li\sdata-reactid=\".*\">some<\/li>/)
end

test 'react_ujs works with rendered HTML' do
visit '/pages/1'
assert page.has_content?('Hello Bob')
Expand Down Expand Up @@ -98,7 +130,7 @@ class ViewHelperTest < ActionDispatch::IntegrationTest
assert_match /data-react-checksum/, page.html
assert_match /yep/, page.find("#status").text
end

test 'react server rendering does not include internal properties' do
visit '/server/1'
assert_no_match /tag=/, page.html
Expand Down