export_state needs new name and format

[sollycatprint]

From @catmando on December 9, 2015 18:18

the export_state macro does two things:

1. creates a state variables that is shared by all instances of the component class.
2. makes the state variable publicly accessible (outside the class) so you can say Foo.state and Foo.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 or shared_state might be the way to go?

Copied from original issue: zetachang/react.rb#97

[sollycatprint]

From @ajjahn on December 9, 2015 22:53

How about a :scope option? Also, I like the idea of the :access option mirroring Ruby's built in attr_reader, attr_writer, attr_accessor terminology. I think it's well understood which of those are creating getters vs setters.

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:

Option	Valid Values	Default Value
:scope	:class, :instance, :shared*	:instance
:access	:reader, :writer, :accessor	:accessor
:initial	Proc, Literal value as evaluated at class evaluation	nil or block if block given

State with scope: :class would initialize the the initial value when the class is evaluated. State with scope: :instance would initialize the initial value before mounting a component instance. *Not convinced scope: :shared is a good idea yet or how we'd determine when to initialize values. Maybe shared state behaves like class state, initializing values at class evaluation, but is just accessible via methods defined on the instance state proxy.

And finally, I think the class level state should be accessed through a state proxy object (currently called StateWrapper), like so MyComponent.state.foo. We should be able to reuse the same mechanism that is providing state for the instance.

[sollycatprint]

From @catmando on December 10, 2015 0:14

1. Not sure what scope: :shared even means? Can you elucidate?
2. Not sure about MyComponent.state.foo. I've thought about it, and I'm unconvinced. For one thing its at odds with ruby accessors. Secondly (and more important) its exposing internal implementation of the class. Lets say you wanted to change the implementation of foo so that it read some other other state, and computed the value of foo.
3. Do like reader/writer/accessor
4. scope :instance - given we have this would we go back to requiring that instance state has to be declared, or would this just be optional?

[sollycatprint]

From @ajjahn on December 10, 2015 2:1

I'm going to address your 4th question first. I do not think state declaration should be optional. In https://github.com/zetachang/react.rb/issues/89#issuecomment-162732528, 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 method_missing because when it's overused, it's a monster. I misspelled 'license' and it didn't care. Later on it happily returned nil and moved on. This is worse than the behavior of plain old local vars. If you ask for the value of a local var that doesn't exist, you get an error pointing to the offending line of code. With method missing, you waste a good chuck of time perplexed, continuing on by breaking some working code trying to fix it. Finally, despair.

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 https://github.com/zetachang/react.rb/issues/97#issuecomment-163426866 is based on the idea that state :foo, ... will build a state proxy object and explicitly define reader/writer methods on it for working with that state. The class would have it's own state proxy, and each instance would have it's own state proxy.

1. scope: :shared would mean that state named baz for example would be defined on the instance state proxy as well as the class level state proxy. Like so:

   class MyComponent
     include React::Component
     state :baz, scope: :shared
   
     def render
       state.baz
       self.class.state.baz
     end
   end
   
   MyComponent.state.baz
   

2. Defining methods on the class itself opens up the possibility of having method name collisions. #83's intent was to solve that problem. Consider this:

   class MyComponent
     include React::Component
     state :param
     param :foo, type: String  # Boom.
     # param is now a class state method, overwriting the param definition method
     #...
   end
   

   For one thing its at odds with ruby accessors.

   I'm not sure I understand what you mean here. Can you explain?

   Secondly (and more important) its exposing internal implementation of the class. Lets say you wanted to change the implementation of foo so that it read some other other state, and computed the value of foo.

   MyComponent.state.foo simply exposes a proxy object with methods that are meant to be exposed. It doesn't actually contain any state. I don't follow how internals are leaking out here.

   You can always define your own foo class method using other state. That's no different than how you'd do it now. You could define or override additional methods on the state proxy object, in the same way, but, that seems really unnecessary. I don't know why you'd chose to do that instead of defining your own class method.

3. Cool.

4. Uh, didn't expect anyone to make it this far. Treat yourself to a small piece of dark chocolate. That's what I'm doing.

[sollycatprint]

From @catmando on December 10, 2015 4:42

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 baz is the same state variable, and it just has two different ways to access it? Or does it mean there is a class level baz that is distinct from all the instance baz variables?

  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 attr_ methods. They are just short hand for declaring methods wrapping instance variables. It would be sad if you had to say some_object_or_class.ivar.foo.

And wrt to the busted param example you can just as easily say:

def self.param
  ...
end

and bust things that way.

Scope and Access are redundant

Just realized we don't need both. All :instance state variables must be read/write.

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 scope altogether, and adding access: :private (or :instance)

Summary

1. All state variables must be declared before access
2. State variables can be declared at the class or instance level. Class level variables have a single state value shared by all instances.
3. By default state variables can only accessed by the state method which is protected and exists for the class and each component instance.

These rules are all that are needed to define state variables, but for simplicity the state macro can be used to define the following additional methods

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

Option	Valid Values	Default Value
:access	:reader, :writer, :accessor, :class, :instance	:instance
:initial	Proc, Literal value as evaluated at class evaluation	nil or block if block given

access :reader, :write, :accessor, :class create a class level state variable.

[sollycatprint]

From @ajjahn on December 14, 2015 3:53

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:

• Instance: Only define state accessors on the instance's state proxy, i.e. state. When instance state updates, only that instance is re-rendered. Instance state's initial values are set before component mount. This is a change from how state is currently initialized.
• Class: Only define state accessors on the class' state proxy, i.e. self.class.state. When class state updates, all that instances of that class are re-rendered. Class state's initial values are set at class evaluation. This is how all state is currently initialized.
• Shared: Just like scope: :class, but also defines accessors on the instance's state proxy. When shared state updates, all that instances of the class are re-rendered. Shared state's initial values are set at class evaluation. This is how all state is currently initialized. scope: :shared most closely resembles export_state behavior as it is currently implemented.

  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 Access

Technically, 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 private inside the singleton class or use private_class_method. Regular old private won't do it:

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:

1. Defining state, whether it be instance or class state, it always define read/write accessors on the respective state proxy, i.e. state.foo, state.foo!, self.class.state.foo, self.class.state.foo!.
2. With no options, default to :instance state.
3. Passing :reader, :writer, :accessor options values defines state accessor methods on the component itself allowing you to avoid typing state. or override them if you change them in the future.

Option	Valid Values	Default Value
:instance	:reader, :writer, :accessor	:none
:class	:reader, :writer, :accessor	:none
:initial	Proc, Literal value as evaluated at class evaluation	nil or block if block given

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!

[sollycatprint]

From @ajjahn on December 14, 2015 4:23

Removing the component :instance accessor method definitions and sticking with state.foo syntax:

Option	Valid Values	Default Value
:class	:reader, :writer, :accessor	:none
:initial	Proc, Literal value as evaluated at class evaluation	nil or block if block given

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!

[sollycatprint]

From @catmando on December 14, 2015 17:4

To be clear on the syntax:

:class can be specified as either class: value, or just :class The :initial key must have a value. but if there is no :initial key the value of the state will be initialized to nil (or to the value of the block)

state :foo, :class, initial: 12 OKAY

state :foo, :class, :initial BAD (its confusing and just makes parsing harder)

[sollycatprint]

From @ajjahn on December 14, 2015 19:25

Yeah, basically state takes an array of arguments. The last argument can be a hash but it's not required. :initial is only a valid key in the options hash not as it's own element in the args array.

Some valid examples with array and hash syntax:

state(*[:foo]) state(*[:foo, { initial: 'biz' }]) state(*[:foo]) { 'baz' }

state(*[:foo, :class]) state(*[:foo, :class, { initial: 'biz' }]) state(*[:foo, { class: :reader, initial: 'biz' }]) state(*[:foo, { class: :writer }]) state(*[:foo, { class: :accessor }]) { 'baz' }

Invalid:

state(*[:foo, :class, :initial]) state(*[:foo, { class: :initial }]) state(*[:foo, :initial])

[catmando]

@zetachang I think this is almost ready to go, and is important as its going to break a lot of existing code, so the sooner we get this in, and the current "define_state" deprecated the better.

The one thing I would like to consider is the possibility of eliminating the :class option in favor of just executing state at the class level as in:

class Todo < React::Component::Base
  class << self
    state :todos, initial: [], access: :reader
    def add(todo)
      state.todos! << todo
    end
    def remove(todo)
      state.todos!.delete(todo)
    end
  end
end

which just follows standard ruby syntax much nicer,

The "access" option would be available to the class method.