-
Notifications
You must be signed in to change notification settings - Fork 35
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
export_state needs new name and format #97
Comments
How about a class MyComponent
include React::Component
state :foo, scope: :class, access: :reader
state :bar, scope: :instance
def render
state.bar
self.class.state.foo
end
end
MyComponent.state.foo So the state definition method would take these options:
State with And finally, I think the class level state should be accessed through a state proxy object (currently called |
|
I'm going to address your 4th question first. I do not think state declaration should be optional. In #89 (comment), I outline many issues I have with it, but just to tack on another, take this example: class IdentificationCard
include React::Component
before_mount do
state.drivers_lisence_number! 'abc-123'
end
def render
span { state.drivers_license_number }
end
end If we were to render this component (using a fictional render method for the sake of this example), this is what we'd expect: React.render_html(IdentificationCard) # => <span>abc-123</span> But this is what we get: React.render_html(IdentificationCard) # => <span></span> WTF? The problem is with Lastly, I'd argue that this... class IdentificationCard
include React::Component
state :name, initial: 'Adam'
def render
span { state.name }
end
end ...is more expressive, concise, and intuitive, than this: class IdentificationCard
include React::Component
before_mount do
state.name! 'Adam'
end
def render
span { state.name }
end
end So my proposal in #97 (comment) is based on the idea that
|
optional vs. mandantory state declarations My biggest hesitation is it is not "ruby" like. However I think we should go ahead and make enforce strict state declarations. You have pointed many good reasons, but even more important right now, is that its easier to "relax" the rules in the future, then decide in a year that we want to enforce strict declarations. Shared scopes So in your example does that mean that state :baz, scope: :shared
state :class_guy, scope: :class
...
def render
state.baz! 12
state.class_guy! 12 # <- is this possible???
self.class.state.baz! 13
state.baz == self.class.state.baz # true? false? ( Either way I don't think we need it. Public Class Level State Variable Access Here is an example of the problem with exposing "state" as a public class level method: class Chat < React::Component::Base
state :online, scope: :class, access: :reader
end ...elsewhere... if Chat.state.online now we do some refactoring class Chat < React::Component::Base
state :chat_handler, scope: :class
def self.online
state.chat_handler.id
end
end ...elsewhere... if Chat.state.online # BUSTED This is analogous in all ways to the And wrt to the busted def self.param
...
end and bust things that way. Scope and Access are redundant Just realized we don't need both. All state :foo, scope: :instance, access: :reader # pointless to have access for :instance vars
state :foo scope: :class # what if I need this to be private but common to all instances? So I propose dropping Summary
These rules are all that are needed to define state variables, but for simplicity the state :foo, access: :reader
# short hand for
state :foo, access: :class
def self.foo; state.foo; end
state :foo, access: writer
# short hand for
state :foo, access: :class
def self.foo!(*args); state.foo!(*args); end
state :foo, access: :accessor
# short hand for
state :foo, access: :class
def self.foo; state.foo; end
def self.foo!(*args); state.foo!(*args); end
# In addition state :foo, access: :class
# Implicitly defines state.foo and state.foo! methods on the instance state proxy object
access |
State Scope:Let me further describe the three different scope options I suggested above so we can be sure we are on the same page as to how each behaves:
state :foo, scope: :instance
state :bar, scope: :class
state :baz, scope: :shared
def render
# Instance only state accessors
state.foo! 1
state.foo # => 1
self.class.state.respond_to?(:foo) # => false
self.class.state.respond_to?(:foo!) # => false
# Class only state accessors
state.respond_to?(:bar) # => false
state.respond_to?(:bar!) # => false
self.class.state.bar! 2
self.class.state.bar # => 2
# Shared state accessors
state.baz! 3
state.baz # => 3
self.class.state.baz # => 3
state.baz == self.class.state.baz # => true
self.class.state.baz! 4
state.baz # => 4
self.class.state.baz # => 4
self.class.state.baz == state.baz # => true
end The more I think about it, the more I think the shared option is a bad idea. I think it should be clear when you are setting class state whether or not other component instances will be effected. # Is this only affecting this instance? Or is this going to affect all instances?
state.foo! 3 So let's throw shared state out. Public Class Level State AccessTechnically, this may not be worth much of a debate due to Opal's all methods are public implementation. In fact, you have to put in some extra effort to make Ruby class methods private; you either need to call So in react.rb, I can access a class' or instance's state proxy from anywhere. This issue really comes down to what we chose to be conventional way of interacting with state. Either way we go, it should be consistent. You're right that it wouldn't make much sense for the instance only state proxy to not have both read & write. It also wouldn't make sense for the class only state proxy to not have both read & write. So with that in mind, I'd take your most recent proposal on step further and suggest the following:
Here's what it looks like with the methods that would be defined: state :foo
# defines:
state.foo
state.foo!
state :foo, instance: :reader
# defines:
state.foo
state.foo!
def foo; state.foo; end # => instance method 'foo'
state :foo, instance: :writer
# defines:
state.foo
state.foo!
def foo!(*args); state.foo!(*args); end # => instance method 'foo!'
state :foo, instance: :accessor
# defines:
state.foo
state.foo!
def foo; state.foo; end # => instance method 'foo'
def foo!(*args); state.foo!(*args); end # => instance method 'foo!'
state :foo, :class
# defines:
self.class.state.foo
self.class.state.foo!
state :foo, class: :reader
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo; state.foo; end # => MyComponent.foo
state :foo, class: :writer
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo!(*args); state.foo!(*args); end # => MyComponent.foo!
state :foo, class: :accessor
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo; state.foo; end # => MyComponent.foo
def self.foo!(*args); state.foo!(*args); end # => MyComponent.foo! |
Removing the component
Here's what it looks like with the methods that would be defined: state :foo
# defines:
state.foo
state.foo!
state :foo, :class
# defines:
self.class.state.foo
self.class.state.foo!
state :foo, class: :reader
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo; state.foo; end # => MyComponent.foo
state :foo, class: :writer
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo!(*args); state.foo!(*args); end # => MyComponent.foo!
state :foo, class: :accessor
# defines:
self.class.state.foo
self.class.state.foo!
def self.foo; state.foo; end # => MyComponent.foo
def self.foo!(*args); state.foo!(*args); end # => MyComponent.foo! |
To be clear on the syntax:
|
Yeah, basically Some valid examples with array and hash syntax:
Invalid:
|
This issue was moved to ruby-hyperloop/hyper-react#97 |
the export_state macro does two things:
Foo.state
andFoo.state!
anywhere.I think we deprecate this, and change it to something like this:
state :state_name, access: :read/:update { optional initializer block }
so you would say
state :foo access: :update { [] }
to create a state called
foo
that was shared by all components of the class, and could be read and written externally as ClassName.foo/foo! (and that is initialized to the empty array)The reason we need to do this is this in many instances you DONT want to export the state or if you do, you only want to export the getter.
Looking for any comments, and in particular suggestions on the syntax.
state
by itself is good because its short, but probably confusing but it does not emphasize the "class level" nature.static
is great but ain't ruby.class_state
orshared_state
might be the way to go?The text was updated successfully, but these errors were encountered: