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

Exception handling #49

Open
wants to merge 7 commits into
base: react-v0.13
Choose a base branch
from
Open
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
21 changes: 14 additions & 7 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ PATH
opal-activesupport (~> 0)
react-jsx (~> 0.8.0)
react-source (~> 0.13)
sprockets (~> 3.1)
sprockets (>= 2.2.3, < 3.1)
therubyracer (~> 0)

GEM
Expand All @@ -16,15 +16,16 @@ GEM
hike (1.2.3)
json (1.8.3)
libv8 (3.16.14.7)
opal (0.7.1)
multi_json (1.11.1)
opal (0.7.2)
hike (~> 1.2)
sourcemap (~> 0.1.0)
sprockets (>= 2.2.3, < 4.0.0)
sprockets (>= 2.2.3, < 3.0.0)
tilt (~> 1.4)
opal-activesupport (0.1.0)
opal (>= 0.5.0, < 1.0.0)
opal-jquery (0.2.0)
opal (>= 0.5.0, < 1.0.0)
opal-jquery (0.3.0)
opal (~> 0.7.0)
opal-rspec (0.4.2)
opal (~> 0.7.0)
rack (1.6.1)
Expand All @@ -42,8 +43,11 @@ GEM
rack-protection (~> 1.4)
tilt (>= 1.3, < 3)
sourcemap (0.1.1)
sprockets (3.2.0)
sprockets (2.12.3)
hike (~> 1.2)
multi_json (~> 1.0)
rack (~> 1.0)
tilt (~> 1.1, != 1.3.0)
therubyracer (0.12.2)
libv8 (~> 3.16.14.0)
ref
Expand All @@ -54,7 +58,10 @@ PLATFORMS

DEPENDENCIES
opal-jquery (~> 0)
opal-rspec (~> 0.4.2)
opal-rspec
rake (~> 10)
react.rb!
sinatra (~> 1)

BUNDLED WITH
1.10.2
8 changes: 8 additions & 0 deletions opal/react/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ def self.included(base)
end
base.extend(ClassMethods)
end

def _render_wrapper
render
rescue Exception => e
message = "Exception raised while rendering #{self.class.name}: #{e}"
`console.error(#{message})` rescue nil
React.create_element("div") # return a valid element so error does not propogate, and we can keep rendering
Copy link
Owner

Choose a reason for hiding this comment

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

How about just return null or undefined?

end

def params
Hash.new(`#{self}.props`).inject({}) do |memo, (k,v)|
Expand Down
1 change: 1 addition & 0 deletions opal/react/component_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def self.native_component_class(klass)
optional_native_alias[:componentDidUpdate, :component_did_update]
optional_native_alias[:componentWillUnmount, :component_will_unmount]
native_alias :render, :render
optional_native_alias[:render, :_render_wrapper]
end
%x{
if (!Object.assign) {
Expand Down
4 changes: 2 additions & 2 deletions react.rb.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ Gem::Specification.new do |s|
s.add_runtime_dependency 'opal-activesupport', '~> 0'
s.add_runtime_dependency 'therubyracer', '~> 0'
s.add_runtime_dependency 'react-jsx', '~> 0.8.0'
s.add_runtime_dependency 'sprockets', '~> 3.1'
s.add_runtime_dependency 'sprockets', '>= 2.2.3', '< 3.1'
s.add_runtime_dependency 'react-source', '~> 0.13'

s.add_development_dependency 'opal-rspec', '~> 0.4.2'
s.add_development_dependency 'opal-rspec'
s.add_development_dependency 'sinatra', '~> 1'
s.add_development_dependency 'opal-jquery', '~> 0'
s.add_development_dependency 'rake', '~> 10'
Expand Down
4 changes: 3 additions & 1 deletion spec/component_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

describe React::ComponentFactory do
describe "native_component_class" do
it "should bridge the defined life cycle methods" do
it "should bridge the defined life cycle methods and the render method" do
stub_const 'Foo', Class.new
Foo.class_eval do
def component_will_mount; end
Expand All @@ -12,6 +12,7 @@ def should_component_update?; end
def component_will_update; end
def component_did_update; end
def component_will_unmount; end
def render; end
end

ctor = React::ComponentFactory.native_component_class(Foo)
Expand All @@ -23,6 +24,7 @@ def component_will_unmount; end
expect(`instance.$component_will_update`).to be(`instance.componentWillUpdate`)
expect(`instance.$component_did_update`).to be(`instance.componentDidUpdate`)
expect(`instance.$component_will_unmount`).to be(`instance.componentWillUnmount`)
expect(`instance.$render`).to be(`instance.render`)
end
end
end
28 changes: 28 additions & 0 deletions spec/exception_handling_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
describe "displaying helpful warning and error messages" do

before(:all) do
%x{
window.old_error_console = console.error
console.error = function(m) { window.last_error_message = m; }
}
end

after(:all) do
%x{
console.error = window.old_error_console
}
end

it "should print and exception message" do
stub_const 'Foo', Class.new
Foo.class_eval do
include React::Component
def render
raise "error happened"
end
end
React.render_to_static_markup(React.create_element(Foo))
expect(`window.last_error_message`).to include("error happened")
end

end
49 changes: 49 additions & 0 deletions upgradetodos.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

observable_spec.rb

native_library_spec.rb <- no big interaction, just hooks into @@component_classes - but depends on RenderingContext

importing native react components (like bootstrap)


export_component_spec.rb <- completely stand alone
export_component


rendering_context_spec.rb
NOTE: rendering context primarily exists so that we can say Foo::Bar in the dsl, because we have no way of knowing the caller, so we have to track it via RenderingContext
NOTE: put the component lookup method in here I think
components can be directly named in the dsl (i.e. if class Foo, then you can just say Foo)
_as_node will return the native node without pushing into the buffer
if element returned from the child block is a string it is added to the buffer
test to make sure subclassing components works

exception_handling_spec.rb
you don't have to have render method, if you just want to export state <- not sure what is going on here, but there is a render method now in the component, that raises an error unless its subclassed
catch and print rendering method if rendering fails.
I'm figuring that there will more exception cases (like misnamed components) that can go in this file.

state_spec.rb
states all get methods - states define methods foo, foo= and foo!
export / import states
states can be defined from within components instances (i.e. before_mount)
define_state extended syntax: strings/symbols are initialized to the default initializer (nil or provided by block), the hash gives name => value pairs
if default initializer excepts params then it is considered a state listener and will receive |state_name, current_value, new_value| on state changes


params_spec.rb
NOTE: probably updates validator
can require param on a single line: required_param or optional_param
can require param of type Proc
params get methods
proc params automatically call
ReactObserver params interact with the observer


depends on react-source <- does it have to (i.e. perhaps for react-rails to work?, if not then do a separate PR at end)

Branch map...

v13_base is synced with v13 BUT already has observable.rb added. (todo if possible, shuffle this around)
<- exception_handling branched fro v13_base
<- export_component branched from v13_base