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

Improve support to ES6 implementation #5

Open
keuller opened this issue Apr 24, 2015 · 3 comments
Open

Improve support to ES6 implementation #5

keuller opened this issue Apr 24, 2015 · 3 comments

Comments

@keuller
Copy link

keuller commented Apr 24, 2015

I'd like suggest an little improvement around RiotControl. I've created a simple application using ES6 syntax, and my store is simple ES6 class, that looks like:

import riot from 'riot';

class ContatoStore {
    constructor() {
        riot.observable(this);
        this.state = [{id:'192', nome:'Abdoral Gusmao', email:'[email protected]'}];

        this.on('contact_add', this.add);
        this.on('contact_remove', this.remove);
        this.on('contact_load', this.load);
    }

    load() {
        this.trigger('contact_change', this.state);
    }

    add(contato) {
        console.log('New Contact');
    }

    remove(id) {
        console.log('Remove contact');
    }
}

module.exports = ContatoStore;

There is nothing wrong with this code below, but RiotControl print out on browser's console Cannot read property 'apply' of undefined. This error message is because el is a function and it's not an object instance. I just did some adjustment on API, putting a validation that shows warning message. But my suggestion is to resolve this issue checking type and instantiating a object in case of function.

['on','one','off','trigger'].forEach(function(api){
  RiotControl[api] = function() {
    var args = [].slice.call(arguments);
    this._stores.forEach(function(el) {
        if (typeof el === 'object')
            el[api].apply(null, args);
        else
            console.warn(el.name +' is not an object instance.')
    });
  };
});
@aMarCruz
Copy link

+1
@keuller, I think using export default class ContatoStore is better than module.exports = ContatoStore. With this, the transpiler can change (at your command) the loader format.

@aMarCruz
Copy link

Confused now.
I'm testing with this todo-store.js file:

'use strict';

import riot from 'riot';

// Storage class

export default class TodoStore {

  constructor(dispatcher) {

    const LOCALSTORAGE_KEY = 'riot-item';

    if (!this.TODO_INIT) {
      this._setEvents();
    }
    this.dispatcher = dispatcher;
    this.storage = window.localStorage;

    let json = this.storage.getItem(this.LOCALSTORAGE_KEY);

    riot.observable(this); // Riot provides our event emitter.

    this.todos = (json && JSON.parse(json)) || [];

    // Event handlers.
    this.on(this.TODO_ADD, item => {
      this.todos.push(item);
      this._triggerChanged();
    });

    this.on(this.TODO_TOGGLE, item => {
      item.done = !item.done;
      this._triggerChanged();
    });

    this.on(this.TODO_CLEAR, () => {
      this.todos = this.todos.filter(item => !item.done);
      this._triggerChanged();
    });

    this.on(this.TODO_INIT, this._triggerChanged);
  }

  _setEvents() {
    const act = ['ADD', 'TOGGLE', 'CLEAR', 'INIT', 'CHANGED'];
    let props = {};
    for (let v, i = 0; i < act.length; ++i) {
      v = 'TODO_' + act[i];
      props[v] = { value: v, enumerable: true };
    }
    Object.defineProperties(TodoStore.prototype, props);
  }

  _triggerChanged() {
    // Brute force update all.
    this.storage.setItem(this.LOCALSTORAGE_KEY, JSON.stringify(this.todos));
    this.trigger(this.TODO_CHANGED);
  }
}

in the main index.js

'use strict';

import riot from 'riot';
import TodoStore from './todo-store.js';
import dispatcher from './riotcontrol.js';
import './tags.js'    // <-- precompiled tags

let todoStore = new TodoStore(dispatcher);
dispatcher.addStore(todoStore);
riot.mount('todo-app', { store: todoStore });

and I have no problems. RiotControl stores intances.

@jimsparkman
Copy link
Owner

TBH I'm not taking advantage of ES6 yet. @duongphuhiep has also suggested #16

I will have to read more into this.

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

No branches or pull requests

3 participants