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

Handle ES6 classes #89

Closed
jhollingworth opened this issue Jan 28, 2015 · 6 comments
Closed

Handle ES6 classes #89

jhollingworth opened this issue Jan 28, 2015 · 6 comments
Labels

Comments

@jhollingworth
Copy link
Contributor

Now that React v0.13 is supporting ES6 classes its only a matter of time before the whole community goes that way. We should support ES6 classes out of the box. Fortunately the concepts in Marty largely map well to classes:

class UserStore extends Store {
  handlers: {
    addUser: UserConstants.ADD_USER
  },
  constructor() {
    this.state = {};
  },
  addUser: function (user) {
    this.state[user.id] = user;
    this.hasChanged();
  }
}


class UserComponentState extends StatefulComponent {
  listenTo: UserStore,
  getState() {
    return {
      users: UserStore.getAll()
    }
  }
}

class User : UserComponentState {
  render() {
    console.log(this.state.users);
  }
}

My major concern is how to map constants to action creators. I believe that the entire action function should have a type associated with it, not just when it dispatches. This was why we went with constants being functions. There have been some issues with this approach (#83) so would like to explore alternative solutions.

One idea I had is to use field annotations. I realise this isn't in a spec but neither is JSX. This would work out the box with Traceur however we need to support 6to5 and people using ES5. My suggestion is writing a annotation specific transformation (e.g. annotationify for browserify, equivalent for webpack and require.js). We would keep the original syntax for those who'd prefer it

class UserActionCreators extends ActionCreators {
  @ActionType(UserConstants.ADD_USER)
  addUser(user) {
    this.dispatch(user);
  }
}

Update

@KidkArolis has half convinced me its a bad idea to introduce annotations (although he agrees it solves the problem well. His suggestion was to call constants from inside the action function. This has the benefit that we don't have to mess with function contexts or introduce new tooling.

class UserActionCreators extends ActionCreators {
  addUser(name) {
    return Constants.ADD_USER((dispatch) => {
      var user = UserUtils.createUser(name);

      dispatch(user);

      return UserHttpAPI.createUser(user);
    });
  }
}

The other possible solution would be to define the types in a hash. On the upside its simpler and follows the same syntax as Stores, on the downside it does require a little mental work to work out which type is associated with an action creator.

class UserActionCreators extends ActionCreators {
  types: {
    addUser: Constants.ADD_USER
  },
  addUser(user) {
    this.dispatch(user);
  }
}
@jhollingworth
Copy link
Contributor Author

@KidkArolis @oliverwoodings would love to hear your thoughts on this

@jhollingworth
Copy link
Contributor Author

In v0.9 branch

@dariocravero
Copy link
Contributor

@jhollingworth could you provide an example of getting some value from a Marty.Store from a Marty.Component?
I've been going through the source and trying to make it work for a couple of hours now and can't seem to discover what am I missing. I'm essentially doing the same thing that's being proposed on the examples:

import Marty from 'marty'
import React from 'react'

export class BlockStore extends Marty.Store {
  constructor(options) {
    super(options)

    this.state = {
      1: {
        data: {
          some: 'data'
        }
      }
    }
  }

  getData(id) {
    return this.state[id].data;
  }
}

export default class Block extends Marty.Component {
  constructor(props, context) {
    super(props, context)

    this.listenTo = BlockStore
  }

  getState() {
    return {
      data: BlockStore.getData(this.props.id)
    }
  }

  render() {
    return (
      <div>
        Block Marty!
        {JSON.stringify(this.state.data)}
      </div>
    )
  }
}

But a call to BlockStore.getData(this.props.id) fails because getData is now an instance method of BlockStore and not a class one.

After reading #153 I have a feeling I'm missing something too.

Could you hint me in the right direction? :)
Thanks,
Darío

@jhollingworth
Copy link
Contributor Author

Sorry, documentation is next on my list :)

We have a new top level API Marty.register(clazz) which will register the class into the internal container and return a new instance of the class. So given your example you would do

class BlockStore extends Marty.Store {
  constructor(options) {
    super(options)

    this.state = {
      1: {
        data: {
          some: 'data'
        }
      }
    }
  }

  getData(id) {
    return this.state[id].data;
  }
}

exports Marty.register(BlockStore)

Marty.createStore is now doing this underneath except we convert the object literal into class before passing it to register.

I'm not entirely happy with this solution but I can't think of a nicer way of doing it... One thing I'm considering is having a browserify transform which automatically wraps anything you export with, e.g. the above would become:

var BlockStore = Marty.register(class _BlockStore { ... })
export BlockStore;

@jhollingworth
Copy link
Contributor Author

@dariocravero
Copy link
Contributor

Class! :) Thanks! The funny thing is that within all that trying I had a call to Marty.register(BlockStore) because from #153 it was clear that register was indeed needed but I wasn't storing that result anywhere :) (thought it was more like a "register this thing here and it will do some magic" kind of thing :), luckily it's not like that. Will chime in with some docs these days too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants