miklschmidt / capacitor.js

MIT License
8 stars 2 forks source link

Use Immutable records in actions and EntityStores. #5

Open miklschmidt opened 9 years ago

miklschmidt commented 9 years ago

The stupid strings are starting to bother me, they aren't used for anything else than checking for uniqueness. I'd like to solve a couple of problems around dealing with actions:

I propose the following:

module.exports = actionManager.create
    fetchUserArticles: null
    fetchUserArticlesSuccess: new Immutable.Record 
        userId: null, 
        items: Immutable.List()
    fetchUserArticlesFail: new Immutable.Record error: null

This would use the key in the action definition object as the unique string. That means the name for that action, has to be unique across all defined actions. Now you can actually search for that name across your entire code base. You know fetchUserArticles doesn't take a payload, and you'll get an error if you try. You know fetchUserArticlesSuccess takes a list of items and a userId, and you can't change any of them in your action handlers.

actionManager.create() would return an object, with the instantiated actions corresponding to each key. Just like it is now.

Further more we should require entity store's to have a Record defined. So each item has to satisfy that record. It could be nice to extend/wrap Immutable.Record so we can define nested properties and maybe even specify types as well.

miklschmidt commented 9 years ago

@kastermester what do you think?

miklschmidt commented 9 years ago

IRL discussion summary: There should be an easier way to define nested properties as well, with a simpler syntax than plain Immutable.js, which is too verbose for that specific use case. Something like:

module.exports = actionManager.create
    fetchUserArticlesSuccess: 
        userId: null
        # EntityStores should have a record that define their items.
        items: ListOf(UserEntityStore.Record) 
    fetchUserArticlesFail:
        # Define a Record on the spot
        error:
            message: null
            status: null
        # Use an existing Record
        error: ErrorRecord
miklschmidt commented 9 years ago

@kastermester It just dawned upon me that this is exactly what React.PropTypes does. It's perfect for this. Plain functions that throw an error if the data doesn't validate, easy to roll your own if needed. We don't even need records.