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

Future of Alt #52

Closed
goatslacker opened this issue Mar 7, 2015 · 25 comments
Closed

Future of Alt #52

goatslacker opened this issue Mar 7, 2015 · 25 comments
Labels

Comments

@goatslacker
Copy link
Owner

This is just to openly discuss any ideas on where you'd like to take alt or what you'd like to do with it. Perhaps actionable tasks can come out of this discussion.

@creatorrr
Copy link

With React v0.13 the use of ES6 classes will be available. Also, according to facebook/react#1380 mixins will not be supported for ES6 classes like they are today with createClass.

So, keeping those in mind,

  1. Are there any plans for supporting alt with ES6 classes once React v0.13 lands (which is coming soon)?
  2. Consideration should be made for alternatives to implement the functionality currently offered via mixins.

@goatslacker
Copy link
Owner Author

You can already use alt with React.Element in v0.13. It seems like mixins are going to be left to be addressed at the language level, if there's a solution that pops up though alt will pick it up and support it.

It was mentioned that there will be first-class support for flux store subscriptions, that's an interesting development.

@bjyoungblood
Copy link

With actions that fetch data asynchronously, it seems to be a pretty common pattern to have associated success and failure actions. It might be worthwhile to explore a way to auto-generate those.

this.generateActions could possibly be modified to support an API similar to Reflux's.

To take this a bit further, I think it's common for an action to create a Promise that calls a corresponding success or failure action with its result. MartyJS achieves this by creating the actions and triggering one of them for you with the result of the promise.

@RnbWd
Copy link

RnbWd commented Mar 12, 2015

Maybe use a mixin pattern like this? https://gist.github.com/sebmarkbage/ef0bf1f338a7182b6775

@creatorrr
Copy link

@RnbWd Interesting but how would this work with multiple mixins overriding the same methods?

@creatorrr
Copy link

@goatslacker Another question for alt is whether it's better to keep it to a small and extensible core or expand it to provide a decent feature set for developing frontend apps. IMHO, the direction of this conversation will depend on that.

@RnbWd
Copy link

RnbWd commented Mar 12, 2015

@creatorrr having multiple mixins overriding the same methods sounds like a bad idea.. honestly I don't know. Do the current mixins override the same methods? I was just passing along a tweet I saw @gaearon post.

Also, haven't tried out all the new mixins yet, are there conflicts with overriding methods? I'm still experimenting within mixin alternatives.. not sure what the pros and cons are yet.

Edit: wrote the same thing twice :P

@gaearon
Copy link

gaearon commented Mar 12, 2015

Interesting but how would this work with multiple mixins overriding the same methods?
having multiple mixins overriding the same methods sounds like a bad idea.

In React, mixins currently have "merging" behavior for lifecycle methods (or mixins wouldn't be useful at all). This is implicit and makes mixin-heavy codebase harder to maintain as it grows.

That "higher level components" gist solves this problem by wrapping components. This way you get lifecycle methods called for free by the virtue of wrappers being real components and thus also receiving lifecycle hooks.

I think Flummox chose right approach: components over mixins. See Why Flux Component is better than Flux Mixin. In the recent release, custom rendering was added, I dig this approach:

let BlogPostPage = React.createClass({
  render() {
    <div>
      <SiteNavigation />
      <MainContentArea>
        <FluxComponent
          connectToStores={{
            posts: (store, props) => ({
              post: store.getPost(props.postId),
            })
          }}
          render={props => {
            // render whatever you want
            return <BlogPost {...props} />;
          }}
        />
      </MainContentArea>
      <SiteSidebar />
      <SiteFooter />
    </div>
  }
});

Gives you full control, and no mixins.

@goatslacker
Copy link
Owner Author

cc @mattmccray who's exploring components over mixins in alt.

@mattmccray
Copy link
Contributor

Yeah, I've just started working with components vs mixins in a new app I'm working on.

I called mine Controller and it has a few API changes from FluxComponent -- Mainly I don't like that render prop, so I have it applying a context to child components. You can see it here: https://gist.github.com/mattmccray/5f3a8cd7935404830a63

It's likely to evolve, and I welcome feedback. So far it's actually been kinda fun to use. That said, I haven't had to do anything hairy with it yet.

@gaearon
Copy link

gaearon commented Mar 12, 2015

Just noting that React has its own concept of context so it may be better to avoid naming confusion. In fact, in React 0.14 it will behave exactly the way you might want for Controller.

@mattmccray
Copy link
Contributor

True! I need to change context to a better name. (Update: I changed it to provide)

Yeah, I'm really interested in parent-based contexts. Primarily for form data management with custom fields, but also this kind of use.

However, there's simplicity in passing props. Passing data in a react context is rather obscure... So I'm not sure if a Controller that passes data via a react context is a great idea.

@acdlite
Copy link

acdlite commented Mar 12, 2015

Yeah, I agree. Context is usually a hack and should be avoided, hence why it's undocumented. Flummox uses it (optionally) to expose the main Flux instance to arbitrarily nested views, but never for passing store state.

@goatslacker
Copy link
Owner Author

@creatorrr late answer to one of your questions. I aim to keep alt core small but provide many components, mixins, and utils.

@pekeler
Copy link

pekeler commented Mar 21, 2015

When Relay is released, how will this impact Alt?

@goatslacker
Copy link
Owner Author

Good question, I don't think something like Relay will replace Flux and there's a good section on it here. I do expect to support the bits of Relay that I can and are useful though.

@troutowicz
Copy link
Contributor

@pekeler
As the link @goatslacker posted explains, Relay is a framework whereas Flux is a front-end design pattern. Relay will be useful for complex applications with many components that require data fetching from a server. Instead of child components depending on data being passed down from N number of parents, the component can statically declare its data dependencies within itself. This will make components more modular and provide a clearer, simpler picture of the needs of each component.

@cpsubrian
Copy link
Contributor

To add to the higher-order-components vs. mixins coversation, I've been using the following utility with alt. Note: this works whether you use ES6 classes or createClass().

connect_to_stores.jsx

import React from 'react'

/**
 * 'Higher Order Component' that controls the props of the wrapped
 * component via stores.
 *
 * Expects the Component to have two static methods:
 *   - getStores(): Should return an array of stores.
 *   - getPropsFromStores: Should return the props from the stores.
 */
const connectToStores = function (Component) {

  // Cache stores.
  const stores = Component.getStores()

  // Wrapper Component.
  const StoreConnection = React.createClass({

    getInitialState () {
      return Component.getPropsFromStores(this.props)
    },

    componentDidMount () {
      stores.forEach(store => store.listen(this.onChange))
    },

    componentWillUnmount () {
      stores.forEach(store => store.unlisten(this.onChange))
    },

    onChange () {
      this.setState(Component.getPropsFromStores(this.props))
    },

    render () {
      return <Component {...this.props} {...this.state} />
    }
  })

  return StoreConnection
}

export default connectToStores

component.jsx

import React from 'react'
import connectToStores from './connect_to_stores'
import myStore from './store'

const MyComponent = React.createClass({

  statics: {
    getStores () {
      return [myStore]
    },
    getPropsFromStores (props) {
      return {
        myProperty: myStore.getState().myProperty
      }
    }
  },

  render () {
    return (
      <div>
        <h1>My Component</h1>
        <p>
          {this.props.myProperty}
        </p>
      </div>
    )
  }
})

export default connectToStores(MyComponent)

If you want to use ES6 classes you can just use regular static methods like:

// ...

class MyComponent extends React.Component {

  static getStores () {
    return [myStore]
  }

  static getPropsFromStores (props) {
    return myStore.getState();
  }
}
// ...

@troutowicz
Copy link
Contributor

@cpsubrian 👍

I really like that approach!

@goatslacker
Copy link
Owner Author

@cpsubrian I wouldn't mind merging this in if you want to submit a PR

@cpsubrian
Copy link
Contributor

Oh cool. I'll work up a PR over the next few days as time allows :)

@cpsubrian
Copy link
Contributor

Posted PR at #115

@srph
Copy link
Contributor

srph commented Apr 8, 2015

@bjyoungblood Has anybody or have you tried this approach?

// Action
fetch() {
  fetchUtil.doSomethin()
    .then((res) => {
      this.dispatch({ data: res.data, err: null });
    })
   .catch((err) => {
      this.dispatch({ err });
   })
}

// Store
handleFetch(result) {
  var { err, data } = result;

  if ( err ) // do something

  // whatever
}

It's just a very simple attempt to simplify success and error handlers. But this has downsides, which I don't acknowledge at the moment.

Awesome approach, @cpsubrian!

@goatslacker
Copy link
Owner Author

Thanks for feedback everyone <3

We've got

@stowns
Copy link

stowns commented Aug 6, 2015

if we're using flux instances with @withAltContext as shown http://alt.js.org/docs/altInstances/ how can we take advantage of connectToStores()? this.props.flux isn't available in the static methods defined on the class. Should these 2 just not be used together?

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