search

goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 321 forks source link

Underdash with martyjs? #337

Open goatslacker opened 9 years ago

goatslacker commented 9 years ago

Opening this ticket to present the thought of merging with martyjs.

I'm thinking marty because both marty and alt have evolved into a state where they share a lot more in common than not so this presents a good opportunity to converge.

Why?

Why not?

This won't be an easy road but we can figure it out.

Thoughts? Concerns?

cc @jdlehman @justjake @spikebrehm

mattkahl commented 9 years ago

I'm fairly new to Flux and while researching different libraries I tried out martyjs (per a recommendation you gave someone in alt's Gitter room). Ultimately, I ended up going with alt (after prototyping with a handful YAFLs).

As I said, I prototyped a feature using martyjs awhile back and if memory serves:

Things I Liked

Things I Disliked

All of that may be of no use. If nothing else, it's the take of an alt user who's tried martyjs (to some extent).

As the 'Underdash' saga has shown, it's probably best to do this sooner rather than later if it's going to happen.

Questions

Shearerbeard commented 9 years ago

Alt and Marty were the two finalists in my search for a Flux implementation to add to an angular codebase at work and to act as the core of a react/typescript framework i'm bootstrapping for my startup.

Marty felt a bit more verbose and opinionated but was a heck of a lot easier to model in a type system like typescript/flow. The class extension vs create functions seems to more closely match the direction the React team is moving and plays great with a type system. This is apparent in their great docs that make the API extremely easy to adapt to. The death nail for me was that Alts dev-tools / time travel seemed much more matures, being able to easily serialize state and step backwards on dispatch will have a huge impact on my future projects. Also Marty seems a bit more closely tied with React. Although my newest project is React based I'm planning on using the same flux implementation to feed Angular and our eventual upgrade to Angular 2 at work as well as React and some WebGL based UI Implementations in my startup. Alt has helper classes for react but seems to generally not care what you put over it, that's a huge win in my book. Also Marty doesn't allow you to customize the dispatcher - I like that you can fire up your own implementation in Alt as I'm interested in experimenting with a csp implementation and maybe tying in some logging by extending Flux dispatcher. I think a merger could be great for the longevity of the project as long as it stays true to Alts UI implementation agnostic attitude. I'll try and publish some of my experimentation/comparison examples from the past month or so to see the differences side by side.

taion commented 9 years ago

cc: @jhollingworth in case he's not already subscribed to this issue

I'd link to the discussion on Reactiflux#general but it's going to scroll off anyway.

The constants thing should be pretty easy to paper over - in principle just add a class decorator that defines constant-like objects on the class to avoid having to go through hoops like: https://github.com/goatslacker/alt/issues/244 (and instead just do something like TodoActions.constants.ADD_TODO or something).

Alt @datasource and Marty Store#fetch are actually really similar, and I think Marty's approach offers slightly leaner implementations, but it's weird unless you're using a base class for Stores.

I don't think it'd be too difficult to offer a migration path/compatibility layer for either of those.

The only part that isn't just porcelain is the different approaches to async. In practice I think https://github.com/goatslacker/alt/pull/284 and http://martyjs.org/guides/isomorphism/index.html offer very similar APIs, in that (if I'm not mistaken) async data loading all goes through a mechanism on Stores that keeps track of all the loads, and only renders once initial loads are done, but the edge cases are going to be different in some ways.

From a docs PoV I think it'd be a nightmare to have a library that supports very similar-looking approaches to isomorphism, but with very different underlying implementations, in a way that errors only arise on somewhat unintuitive edge cases. I understand it's good to be unopinionated, but I feel like it'd be a disservice to users not to just choose one, because the potential confusion would not justify the flexibility.

taion commented 9 years ago

And as a minor technical bikeshedding thing, I think pure HOC containers with static connections to stores are a bit cleaner implementation-wise and makes it more clear that store connections aren't really like normal props - or shouldn't really be treated as such anyway.

jhollingworth commented 9 years ago

First off, :+1: to this.

From what I've seen of the Alt API most differences between Marty & Alt are largely cosmetic so I don't have too many concerns with breaking changes.

First question I have, do you see this as merging of the code bases under one of the existing projects or creating a new project which aims to have an easy migration path from Marty & Alt apps?

dkingman commented 9 years ago

I'm in the process of writing a large app and I started out with Marty. I moved to Alt after a 6-8 weeks. Marty is great for a lot of use cases but it was too opinionated for my app. Alt cut down on some of the boilerplate for me and was generally easier to customize behavior.

The other reasons that made me move to Alt:

Just to be clear, I think Marty is a pretty good project, but apart from combining contributor and user base's (which may be sufficient reason) I don't see much benefit from merging.

taion commented 9 years ago

@dkingman

Definitely share your pain with error handling and boilerplate in Marty. That said, those things are largely porcelain - constants are just strings, queries and action creators can be merged, and the main use case for state sources as a concrete abstraction are to do with transparently changing how they behave on the server-side. I don't think those are large barriers for migrating from a Marty setup to an Alt setup, and I do think the patterns there in Alt are better.

From my POV, there are two main benefits to merging the projects:

1) The "hard stuff" of dealing with async data loading and making it as easy as possible to do isomorphic rendering - it'd be really great to merge the best ideas from both frameworks into something that's hopefully better and gives users easier ways to solve their issues.

2) For building higher-level abstractions, it's helpful to have fewer libraries to target. I'm working on something to make it very easy to connect to REST resources with minimal boilerplate (i.e. not even manually setting up actions and stores) that I think will be useful to others, and having a single framework to target there is a good thing for my sanity.

goatslacker commented 9 years ago

@swordsreversed this is not the right thread for that discussion. Feel free to join the gitter room or slack channel if you want to discuss it.

@mattkahl I did speak to them on Reactiflux briefly. I haven't sat down to think about what do we want to change in Alt and Marty in the merge, I do have some high-level goals in mind though:

@Shearerbeard making Alt easier to type would also be a plus as well as making the boilerplate optional.

@jhollingworth this is mostly a matter of logistics:

Creating a new separate org/repo

Pros:

Cons:

Moving into Alt

Pros:

Cons:

Moving into Marty

Pros:

Cons:

We don't have to make these decisions now, first we should probably figure out what we want to do which will then inform what we want to call this.

Initial Thoughts

taion commented 9 years ago

@goatslacker

I'd prefer a merge into the more popular repo/package, just to save as much migration pain as possible.

Marty queries are just action creators; they were initially set up to solve a problem with circular dependencies across modules that largely isn't an issue any more AFAICT, and definitely isn't an issue any more if you're using instances.

Marty's state sources are actually more analogous to vanilla Flux web API utils, and they let you do cool things like handle cookies differently when rendering on the server, or rewrite URLs for HTTP requests on the server. I think they're a useful abstraction for isomorphic rendering, but they're better off split into a utility library. Not much benefit to defining an entire new class just to call fetch for me without that context.

I believe this part is most analogous to the async logic in Alt data sources - http://martyjs.org/guides/fetching-state/index.html - and I think this might be the one part of Marty that has less boilerplate than Alt (heh), since you're just calling this.fetch from a getter method on the store, rather than setting up a separate data source object.

The local/remote stuff is mostly the same. The big difference is that the pattern in Marty is to call into an action creator on the remote branch, and dispatch an action from there (on completion) that the store then handles. I think it's nice to go through an action and the dispatcher for this, rather than having the store update itself directly. However, this doesn't really work if you don't have constants separate from action creators, because this requires the store to both call into action creators and be able to handle actions identified by constants associated with those action creators. Not sure how to resolve this.

EDIT: Crossed out section that was completely wrong.

justjake commented 9 years ago

I like the direction that marty's data fetching is going in, but i don't like to write more files. I would like to see even more behaviour for optimisitc updates and cache invalidation, like @dkingman mentioned in his reply. No one in Fluxland seems to be hitting the datafetching and API interaction sweetspot yet.

I'd like to see stores / apis that:

Shearerbeard commented 9 years ago

@goatslacker I think keeping constants but hiding them optionally wouldn't be a hard implementation - we would be basically porting convenience methods like generateStore to implement these for us and abstract the pain away. Hopefully this wouldn't be at the cost of to much code size.

taion commented 9 years ago

@justjake

I think what you're talking about is best handled with a library built on top of a Flux framework, as opposed to part of a Flux framework itself. I tried to allude to some similar things. However, it's much easier to build/maintain/make useful such a library if there are fewer Flux frameworks around, which a merger will help serve.

It sucks to be in a position where you really want library A from framework 1 and library B for framework 2, but can't use them both. I'm axed to have as few people feel that pain as possible.

taion commented 9 years ago

@Shearerbeard

Just Alt's implicit constants would be sufficient, no? http://alt.js.org/docs/actions/#actionconstant

I thought there might have been weird circular dependency issues, but on second thought there aren't.

justjake commented 9 years ago

@taion yeah, I think you're right about caching/proxy/request state stuff being usable across multiple flux implementations.

@goatslacker when you look at the Marty APIs, which ones do you see as most important to bring to Alt?

goatslacker commented 9 years ago

I highlighted them above:

Take Marty's instances. Take Marty's class extends Store approach (better for typing and tooling). Take Marty's HoC (Marty.createContainer and all of its really nice methods) for connecting stores. Merge Queries and Sources into just DataSources. Keep constants but hide them away optionally. Adopting Marty's git commit messages, and update them with the latest from atom The diagnostics from server rendering The TestUtils, expanding them with methods like hasDispatched, etc...

I still have to do some more digging.

Shearerbeard commented 9 years ago

Another thought that I noticed porting a work project over to Alt today. Marty's State sources adapt a bit more cleanly to server sent events like Websocket communication. It provides a clear path for binding external events with action creators. Data Sources only seem to work well for an asynchronous request model and I could not find a natural fit in the library to translate socket subscriptions to actions unless I'm missing something in utilities.

taion commented 9 years ago

I really don't think data sources and queries/state sources map to each other that well. Queries are exactly the same as action creators and were there to get around a circular import issue in (I think) an older version. State sources are an abstraction around web API utils that let you do things like: https://github.com/martyjs/marty-express/blob/ce5d367248af111e72493da36708e512ca9bf53b/index.js#L25-L31

I guess one question is - is it a lot better to specify loading/success/error actions on the data source, v. invoking an explicit action creator method that dispatches those methods for you? I think most of my local/remote bits in stores do follow that pattern, but... well, I'm not totally sure yet.

I think in terms of big breaking changes (net of compatibility wrappers) for users, we have something like the following:

Marty users:

Alt users:

Thinking a little bit more about instance vs bootstrap/flush-based isomorphism, I'm still inclined to say we should kick one of them out. I feel like with a little bit of better warning/logging around making sure that all data loads go through #284-like mechanisms, and not permitting stores to handle actions out of the times when we've gathered all of our async data loads could prevent most potential errors, and it's just easier for everyone involved if there's only one way to do things.

I feel that it's really difficult to optimize for doing both equally well, and it'd be most useful if there were as few separate patterns as possible.

goatslacker commented 9 years ago

Does Marty have any releases coming up? Anything changing?

alt@0.17 has been merged into master but hasn't been released to npm because I was going to docs overhaul. I'm wondering if I should just skip this release until after underdashing.

jdlehman commented 9 years ago

I like the idea of this merge a lot. I think there are things that Marty and alt both do well, and merging the two together would get those benefits as well as a larger community of contributors (which is almost an even bigger win especially with the large number of Flux libraries out there).

My gut feeling is that it that while it might not be difficult to merge one library into the other, it may put the library in a weird state as far as docs and code that is not necessarily needed anymore. I am in favor of creating something new as I am sure there are things each codebase would have done/architected differently in hindsight, though this is probably the approach with the most work required and like @goatslacker mentioned in his pro/con lists above, it could hurt adoption as we would just be throwing another Flux library into the mix with the hundreds of others that already exist.

So while I think creating something new (malt or whatever we want to call it) has some unique benefits, I think it is also viable to just pick one and make changes to get it into the state we want it to be in. As far as picking which one we would make the "primary"/new library, we should probably use the one that has a codebase most like where we want to end up. Since it sounds like we want to go with the class extends Class approach of Marty, it might make more sense to use that as our starting point (if we choose to take this strategy rather than building something new). Though maybe this change isn't as bad as I think it might be in alt, so maybe it does make sense to go with the more popular repo, alt (based on number of npm downloads).

I also have a feeling there will be a number of people that would rather continue using alt or Marty as is (though I suppose they could always fork and continue the project that way), so I am not as concerned with that being a huge issue.

taion commented 9 years ago

@goatslacker

Marty v10 was a big breaking change release, and only a month ago. I don't think there are any further such changes coming up (and I wish we were having this discussion before that release... forcing users to make major migrations twice on consecutive releases is sorta sucky).

@jdlehman

https://xkcd.com/927/ (s/standard/framework)

I agree with a lot of what you're saying. Where do you think the specific pain points with duplicated libraries/code would be?

goatslacker commented 9 years ago

Ok cool, so plan of action...

Also, I don't know how busy @jhollingworth is or what he can take on.

I don't think there will be much pain points in merging into alt since alt is kinda small.

Just a thought: Alt can be the core (marty-lib) and Marty can be the react collection which includes the HoC and other blessed tooling by default?

taion commented 9 years ago

FWIW I like the approach you took with Alt's web and native support both being in https://github.com/goatslacker/alt, rather than having https://github.com/martyjs/marty-lib, https://github.com/martyjs/marty, and https://github.com/martyjs/marty-native. For one thing, I have no clue where to open issues for Marty any more, even if it's only for my own benefit...

Maybe a very simple first step would be to see how hard it would be to write a compat wrapper that lets me e.g. map the basic action/store stuff from Marty to Alt, if we decide we don't want to explicitly have constants and just merge actions creators and constants like Alt does.

I think containers/&c. (HoC or otherwise) should probably be part of the core library because otherwise it's just painful for users. As @jhollingworth said, state sources are probably better off living in a separate blessed package - they have nothing to do with Flux per se, they're just a nice way to allow users to write isomorphic code in cases where they need slightly different server-side behavior.

I think the really big conceptual question is what to do w/r/t https://github.com/goatslacker/alt/pull/284 and the Marty equivalent. Having the framework gather together all of your remote data fetches makes life so much easier for server-side rendering, but it feels wrong to have both an implementation that works with singletons and bootstrap/flush as well as a separate one that works with instances.

Maybe the implementations will be similar enough or share most of the code, but I'd have no idea how to even start explaining it in docs.

jhollingworth commented 9 years ago

I don't know how busy @jhollingworth is or what he can take on.

Becoming a lot less busy, I've got time to commit to this

Does Marty have any releases coming up? Anything changing?

No major releases planned right now. The only major thing I was thinking of changing in v0.11 was to depracate Queries (Superfluous now, just use action creators) & state sources.

The motivation for introducing state sources was so we can easily modify their behaviour on the server (e.g. Getting a CookieStateSource on the server will look at the req object rather than document.cookie). While I think the idea is still sound, I think this would be better as a seperate library rather than inheriting from a class

let iso = require('malt-iso')

iso.cookies.get('foo')
iso.http.get('/foo').then(res => ...)

Take Marty's instances

Do you mean Application?

I'm still not 100% how we should deal with instances. In most cases people just want a single application instance and they shouldn't have to take on the complexities of multiple instances. The problem I found was you'd then you tend to have to support a second API just for isomorphisim. This meant two APIs to document, bug fix, etc. In v0.10 we decided it would be better to just always assume you're going to have multiple instances and make it as easy as possible to deal with.

I've always had mixed feelings about Alt's approach to instances, i.e.

let alt = require('../alt')

module.exports = alt.createStore(...)

On the one hand, its really simple and easy to understand. On the other, I've found separating the class from registration opens up more scenarios. e.g. We've got a bunch of components (with associated stores/action creators) which live inside their own packages that we just require in. This sort of thing becomes a lot harder when you need to pass the actual instance in.

class App extends Application {
    constructor () {
        this.register(require('foo'))
    }
}

//foo/index.js

module.exports = {
    foo: {
        fooStore: require('./stores/fooStore')
    }
}

That said, I've always admired Alt's simplicity and I think that should be a guiding principle so very happy to adopt a different approach.

I like the approach you took with Alt's web and native support both being in https://github.com/goatslacker/alt, rather than having https://github.com/martyjs/marty-lib, https://github.com/martyjs/marty, and https://github.com/martyjs/marty-native.

Yeah, this really annoyed... I felt a bit dirty about the package.json saying it had a peerDependency on react when it wanted react-native. It also meant that whenever I wanted to reference React I had to do this:

try {
    return require('react')
} catch (e) {
    return require('react-native')
}

I'm not too precious about either of these points though so happy to just have a single repo.

Alt can be the core (marty-lib) and Marty can be the react collection which includes the HoC and other blessed tooling by default?

Makes sense. It would be nice to keep React out of the core lib so we could support other libs.

The only concern I have is, unless I'm missing something, the only concepts we're going to support are

Is it it worth us having a seperate package just for HoC?

I think the really big conceptual question is what to do w/r/t #284 and the Marty equivalent. Having the framework gather together all of your remote data fetches makes life so much easier for server-side rendering, but it feels wrong to have both an implementation that works with singletons and bootstrap/flush as well as a separate one that works with instances.

I agree, would be interested in your thoughts on this. Also, do you think its worth still maintaining Marty's express.js middleware

How are we going to deal with action creators?

So far the only major difference I see between Alt & Marty is how we dispatch actions.

Alt has a one-to-one mapping between functions and their constants where as Marty expects the constant as the first argument.

doSomething() {
    //alt
    this.dispatch('foo', 'bar')

    //marty
    this.dispatch('DO_SOMETHING', 'foo', 'bar')
}

My questions are

This was the approach that came to my mind, thoughts?

class UserActionCreators extends ActionCreators {
    static martyDispatch = true
    static constants = ['CREATE_USER', 'REMOVE_USER']

    createUser (user) {
        this.dispatch(this.CREATE_USER, user)
    }
}

Maybe a very simple first step would be to see how hard it would be to write a compat wrapper that lets me e.g. map the basic action/store stuff from Marty to Alt, if we decide we don't want to explicitly have constants and just merge actions creators and constants like Alt does.

I agree, maybe its worth us trying to document the API in a markdown document somewhere? Should help clarify if there are any major issues

taion commented 9 years ago

@jhollingworth

https://github.com/goatslacker/alt/pull/284 shows a pretty cool way to do isomorphism with singletons, though. You gather all your remote data loads, wait for them all to complete, dispatch them into stores all at once, then clean up the stores when you're done. Implementation-wise this is a bit more complicated than using per-request application instances, but as long as you can enforce that all data loads go through a mechanism that you can instrument/capture, can potentially provide for a much nicer experience for users, who now no longer have to worry about context.

I can't stress enough how much I think Marty's Store#fetch makes my life easier, though, and I don't think you can really provide something like this without fairly tight integration between store-fetches and container HOCs, so I really think those have to stick around.

The action dispatch thing is a bit of a tough one, though. The things I really like about Alt's syntax are that:

  1. It makes it impossible to dispatch something that isn't an action, so it forces you to be more static/explicit
  2. It has less boilerplate.

My main concern is that, to me at least, it's really confusing to define a method on a class that calls this.dispatch, where in that context this presumably isn't the Actions object itself. I can't think of a better way to do it, but it's strange to me. I'd almost rather call this.GET_USER.dispatch(payload) or whatever, but of course that is extra boilerplate...

taion commented 9 years ago

Looking at the actions thing a bit closer, it looks like maybe the idiomatic way to handle actions with Alt differs from with Marty, e.g.

I see a few examples of Alt actions that do some complex behavior, but it seems like largely they don't.

Again looking at the Alt examples, though, if I'm not mistaken, most of them seem to just use this.generateActions in the ctor, and generally don't attach much behavior to most of the actions, and so treat them more like dispatchable constants.

Maybe the API can look something like:

class UserActions extends Actions {
  constructor(options) {
    this.generateActions('GET_FOO_STARTING', 'GET_FOO_DONE', 'GET_FOO_FAILED');
  }

  getFoo(id) {
    this.GET_FOO_STARTING({id});
    return FooApiUtils.getFoo(id)
      .then(
        result => this.GET_FOO_DONE({id, result}),
        error => this.GET_FOO_FAILED({id, error})
      );
  }

  // and maybe... ?
  @action
  DELETE_FOO(id) {
    this.dispatch(id);
    // do some extra work
  }
}

In other words, separate out generated actions (constants) from action creator methods, and maybe have a convention that we use CONSTANT_CASE for the former and camelCase for the latter?

This is pure bikeshedding, though. I don't think these meaningfully affect architecture, or what it would take to write a compat shim.

jhollingworth commented 9 years ago

284 shows a pretty cool way to do isomorphism with singletons

Yeah, I tried a similar approach earlier on. I found it didn't scale very well because the components data requirements are no longer encapsulated. Whenever you add/remove a component you have to remember to update the fetching logic which is a maintenance overhead and introduces a whole class of bugs. This becomes much worse when a component needs the response of an earlier API call to do its own fetches (Very common). Also, you now have to implement handlers for each view. Not only is this a pain but you also have to duplicate your routing logic.

app.get('/foo', (req, res) => {
    AltIso.define((props) => {
      return Promise.all([
        userStore.fetchUser(props.id, props.name),
        numberStore.fetchNumber(props.id)
      ])
    })

    AltIso.render(alt, User, { id: 0, name: 'ZZZZZZ' }).then((markup) => {
        res.send(markup)
    })
})

Marty's approach is to

  1. Render the component
  2. Wait for any remote fetches to finish.
  3. Repeat 1 & 2 until no more remote fetches are started
  4. Render the component again and return

The major benefit here is you don't have to know what data each component needs. You just keep on re-rendering until it doesn't ask for anything more. Now that you only need one single, generic handler, you can do neat things like re-using your react-router routes on the server.

I see a few examples of Alt actions that do some complex behavior, but it seems like largely they don't.

Where does the more complex logic tend to go in alt apps then?

@taion

I like that API you proposed. Reminds me a bit of how constants looked in Marty v0.8 :)

taion commented 9 years ago

@jhollingworth

I think you can combine the best of both for isomorphic rendering.

  1. On initial request, synchronously:
    1. Initialize stores.
    2. Do initial render.
    3. Intercept all remote fetches at application level.
    4. Save data in stores.
    5. Wait for all remote fetches to complete.
  2. On completion of remote fetches, synchronously:
    1. Re-apply saved store data.
    2. Apply remote fetch results.
    3. Re-render.
    4. If there are more fetches:
      1. Repeat 1 iii through v.
    5. If there are no more fetches:
      1. Invoke callback (to send to user).
      2. Clear stores (maybe optional, and then we can just return a future?)

The trick for this to work is that instead of invoking a generic action creature method, the equivalent of the fetch object would have to specify which constant to dispatch when complete, like Alt's data sources - then you can just dispatch all the completions (or failures) simultaneously from the application's render loop.

This is a little more restrictive than what Marty currently does in allowing you to call an arbitrary action creator method, but I think this captures all of the use cases I have with remote data fetching, and has a bit less boilerplate as well, since you can define everything on the data source object itself.

Repeatedly initializing and clearing stores is a little annoying, but the tradeoff is that you no longer have to care about instances or context at all, and can just go back to using singletons, so issues like not being able to access your stores in willTransitionTo go away.

taion commented 9 years ago

The trick for this to work is that instead of invoking a generic action creature method, the equivalent of the fetch object would have to specify which constant to dispatch when complete, like Alt's data sources - then you can just dispatch all the completions (or failures) simultaneously from the application's render loop.

On second thought, you could invoke generic action creator methods from e.g. the remotely branch of a fetch object. You set up the action creator object to intercept all calls to the action creator, and then before calling into those methods, check if e.g. the app has a dispatch buffer set up (for the current request). If so, invoke the action creator methods with this set to an object where the dispatch just sits in a buffer (to be later drained) rather than going directly to the dispatcher.

Then this gets appropriately passed through to all the future continuations such that anything chained will get buffered, and we can drain the buffers once all futures complete.

I can't really think of any major benefits to doing things this way (I guess if you ever need to manually invoke the "get data" action creator, you have that option), but, uhh, it's possible.

taion commented 9 years ago

Refining the actions proposal from Gitter convo, this is a part yak shaving and part bikeshedding but here goes -

Actions/constants in Alt generally look very nice to work with due to the reduced boilerplate, but my main concern is that Actions classes implement idiosyncratic behavior to accomplish this that makes them not resemble normal ES6 classes, and behave in ways that may be surprising or unclear to users who aren't familiar with Alt patterns.

I'd like for actions to both offer a terse, low-boilerplate syntax, while still following the lines of more standard ES6 objects, with more intuitive behavior for this.

I propose to have 3 categories of properties on an Actions object;

  1. Standard actions that just forward their payload
  2. Actions that transform and forward their payload
  3. Normal methods that aren't actions at all (but may serve as action creators that dispatch other actions)

I think the code should look like this:

class FooActions extends Actions {
  static actions = ['simpleAction', 'getFooStarting', 'getFooDone', 'getFooFailed'];
  // Or maybe this.generateActions(...) in constructor.

  @action
  actionWithTransform(value) {
    return this.getTransformed(value);
  }

  getTransformed(value) {
    return 2 * value;
  }

  getFoo(id) {
    this.getFooStarting({id});
    FooWebApiUtils.getFoo(id).then(
      result => this.getFooDone({id, result}),
      error => this.getFooFailed({id, error})
    );
  }
}

What you end up with is that FooActions.simpleAction is just a simple action that will dispatch its payload, and that FooActions.actionWithTransform is an action that will apply a simple transform to its payload before dispatching it.

However, FooActions.getTransformed isn't an action at all, and will just return the transformed value. Likewise, FooActions.getFoo is not an action - it's a standard Flux-y action creator that dispatches other actions (by calling their methods).

My concerns with this proposal are:

  1. Is it sufficiently obvious that e.g. above, getFooDone is an action but getFoo is an action creator? Per my earlier proposal, maybe use CONSTANT_CASE for actions and camelCase for regular methods?
  2. This is proposal is incompatible with some Alt patterns - if you have an action that looks like e.g. foo(value) { fooFuture().then(() => this.dispatch(value)); }, you would need to create a new e.g. fooDone action and dispatch that (via this.fooDone(value)), as fooDone, rather than as foo as it would be right now. This is the flip side of having a firm separation between actions and action creator or helper methods.
goatslacker commented 9 years ago

We should probably split these issues up so it makes it easier to follow each development individually. I like that though.

taion commented 9 years ago

@goatslacker Makes sense. Do you want to use this repo's issue tracker, or should we make another repo to host the issues to limit noise here?

goatslacker commented 9 years ago

This repo is fine. I'm guessing the merge will happen in this repo? If so then it's ok to have the noise since these are issues that need to be resolved regardless.

taion commented 9 years ago

I'll open some issues then. Maybe it would be good to have a label for them.

taion commented 9 years ago

In checklist form:

jhollingworth commented 9 years ago

hey, sorry I've been a bit quiet of late... So you know, I've done a flummox and started using redux. Marty v0.10 will be the last Marty release. I will recommend everyone more to alt/redux moving forwards!

taion commented 9 years ago

Redux, killer of Flux frameworks :D

I think the Marty -> Alt migration path is going to be the easier one once we find time to sort out the remaining issues around render loops and stuff.

ariddell commented 9 years ago

Is the clear recommendation for marty.js users to migrate to Alt once these issues are addressed -- or is it just a suggestion? Will migrating to redux be significantly harder?

taion commented 9 years ago

It depends on which features of the framework you're using.

ariddell commented 9 years ago

Ok, sorry for the noise. I've read the redux docs and can see it's rather different. I'll give it a shot.

Marty.js was great while it lasted!

On 09/04, Jimmy Jia wrote:

It depends on which features of the framework you're using.

Reply to this email directly or view it on GitHub: https://github.com/goatslacker/alt/issues/337#issuecomment-137745640

jhollingworth commented 9 years ago

I moved a fairly large Marty app to redux in a day. Most differences are largely cosmetic

On 4 Sep 2015, at 8:08 pm, Allen Riddell notifications@github.com wrote:

Ok, sorry for the noise. I've read the redux docs and can see it's rather different. I'll give it a shot.

Marty.js was great while it lasted!

On 09/04, Jimmy Jia wrote:

It depends on which features of the framework you're using.

Reply to this email directly or view it on GitHub: https://github.com/goatslacker/alt/issues/337#issuecomment-137745640 — Reply to this email directly or view it on GitHub.

dcousineau commented 9 years ago

@jhollingworth Are you planning on writing up a brief writeup on that? I have a largeish Marty.js app that I need to migrate to something and would love to read about any pitfalls you might have hit during your migration and any tips you have...

pbomb commented 9 years ago

@jhollingworth I second that proposal. I would love to see a writeup on the migration. The two frameworks seem pretty different, so I'm curious how you migrated the stores to reducers and how you handled fetching data. We have a very large repository as well that we might need to migrate soon note that React 0.14 is imminent.

bishtawi commented 9 years ago

@jhollingworth I would appreciate a writeup on how you were able to move a nontrivial Martyapp to redux (or alt). It would be a great way to sunset Marty and help your userbase transition to another flux implementation. I have a fairly large Marty app that I would like to move to an in-development flux framework.

dcousineau commented 9 years ago

Even just a bulleted list of things to watch out for and in general where I should map Marty features to in the new framework would be awesome and help with confidence levels going into the transition

jhollingworth commented 9 years ago

Hey, sorry for the delay. I've just written up translation from marty to redux https://gist.github.com/jhollingworth/ed66357097451fb2a58f.

I hope it helps