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

Underdash with martyjs? #337

Open
goatslacker opened this Issue Jun 18, 2015 · 45 comments

Comments

Projects
None yet
@goatslacker
Owner

goatslacker commented Jun 18, 2015

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?

  • Get more contributors on board.
  • Better documentation.
  • More examples.
  • Increase usage of the library.

Why not?

  • Because we might have to make some breaking changes.

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

Thoughts? Concerns?

cc @jdlehman @justjake @spikebrehm

@mattkahl

This comment has been minimized.

Show comment
Hide comment
@mattkahl

mattkahl Jun 18, 2015

Contributor

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

  • The documentation was stellar. The classic/ES6 toggle on examples is a great touch.
  • At the time, it had a more fleshed out async pattern than alt with things like State Sources. Since then, alt has added Data Sources.
  • Being new to Flux, I appreciated the opinionated pattern of Queries vs. Action Creators.

Things I Disliked

  • It felt like a lot more boilerplate. I really appreciate alt's generateActions as an example of eliminating boilerplate. I can see (and potentially buy) the argument for separating Constants and Action Creators, but it sure does lead to a lot of redundancy for many use-cases.
  • I'm currently not working in an isomorphic JS environment, and (rightfully so) martyjs definitely works hard to support it. But, the documented patterns didn't feel as conducive to the non-isomorphic, non-React, non-webpack environment that I'm in. So, when evaluating Flux libraries, that gave me pause.
  • This isn't a dislike of martyjs, but an endorsement of alt: The alt Gitter room is extremely active, helpful, and supportive. I would argue even more so than martyjs.

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

  • Out of curiosity: Have you spoken to the martyjs team about this, or is this the start of the conversation?
  • What martyjs patterns/paradigms would you want to avoid if a merge did happen?
Contributor

mattkahl commented Jun 18, 2015

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

  • The documentation was stellar. The classic/ES6 toggle on examples is a great touch.
  • At the time, it had a more fleshed out async pattern than alt with things like State Sources. Since then, alt has added Data Sources.
  • Being new to Flux, I appreciated the opinionated pattern of Queries vs. Action Creators.

Things I Disliked

  • It felt like a lot more boilerplate. I really appreciate alt's generateActions as an example of eliminating boilerplate. I can see (and potentially buy) the argument for separating Constants and Action Creators, but it sure does lead to a lot of redundancy for many use-cases.
  • I'm currently not working in an isomorphic JS environment, and (rightfully so) martyjs definitely works hard to support it. But, the documented patterns didn't feel as conducive to the non-isomorphic, non-React, non-webpack environment that I'm in. So, when evaluating Flux libraries, that gave me pause.
  • This isn't a dislike of martyjs, but an endorsement of alt: The alt Gitter room is extremely active, helpful, and supportive. I would argue even more so than martyjs.

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

  • Out of curiosity: Have you spoken to the martyjs team about this, or is this the start of the conversation?
  • What martyjs patterns/paradigms would you want to avoid if a merge did happen?
@Shearerbeard

This comment has been minimized.

Show comment
Hide comment
@Shearerbeard

Shearerbeard Jun 18, 2015

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.

Shearerbeard commented Jun 18, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

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: #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 #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.

Collaborator

taion commented Jun 18, 2015

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: #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 #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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

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.

Collaborator

taion commented Jun 18, 2015

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

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Jun 18, 2015

Collaborator

First off, 👍 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?

Collaborator

jhollingworth commented Jun 18, 2015

First off, 👍 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

This comment has been minimized.

Show comment
Hide comment
@dkingman

dkingman Jun 18, 2015

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:

  • Exception swallowing throughout the code, creating difficult debugging conditions.
  • As mentioned earlier in this thread, very verbose. Every time I needed to add a data component I would have to create a file for:
    • constants
    • actions creator
    • queries
    • state source
    • store
  • In contrast, with Alt I have
    • store
    • api
    • actions
  • State management becomes difficult when pagination and cache invalidation get involved. Alt doesn't really solve these problems either, but it leaves more room to implement custom solutions

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.

dkingman commented Jun 18, 2015

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:

  • Exception swallowing throughout the code, creating difficult debugging conditions.
  • As mentioned earlier in this thread, very verbose. Every time I needed to add a data component I would have to create a file for:
    • constants
    • actions creator
    • queries
    • state source
    • store
  • In contrast, with Alt I have
    • store
    • api
    • actions
  • State management becomes difficult when pagination and cache invalidation get involved. Alt doesn't really solve these problems either, but it leaves more room to implement custom solutions

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

@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.

Collaborator

taion commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 18, 2015

Owner

@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:

  • The library needs to be small (code size in bytes) and have a small API surface layer. Alt tries to abide by this practice and leaves most of the implementation specifics to addons or userland. Some dependencies may need to be shed. Alt currently weighs 5.9k gzip.
  • Server rendering is important. We should pick something that makes sense and go with that.
  • Flexibility: allowing you to customize various parts.
  • Better documentation. There are lots of things I love about Marty's docs so I'd like to see all of the code documented (including addons) along with guides and plenty of examples.

@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:

  • Leaves alt and marty unchanged.
  • We can potentially backport features where alt and marty are just specialized builds (alt unopinionated and marty with more opinions?)
  • We can name the frankenflux: malt which is like a tasty beverage.

Cons:

  • Will probably hurt adoption. This is a big one.

Moving into Alt

Pros:

  • This repo has more stars and npm downloads (25k/month)
  • It already has a wide adoption.
  • It is not named after something from back to the future.

Cons:

  • Googling for alt or altjs kinda sucks.
  • We would be renaming the marty org.

Moving into Marty

Pros:

  • It already has a wide adoption.
  • martyjs is easier to search for.

Cons:

  • The alt repo and npm name would be abandoned.
  • We are now named after this dude.

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

  • Take Marty's instances.
  • Take Marty's class extends Store approach (better for typing and tooling).
  • Take Marty's HoC for connecting stores.
  • Merge Queries and Sources into just DataSources.
  • Keep constants but hide them away optionally.
Owner

goatslacker commented Jun 18, 2015

@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:

  • The library needs to be small (code size in bytes) and have a small API surface layer. Alt tries to abide by this practice and leaves most of the implementation specifics to addons or userland. Some dependencies may need to be shed. Alt currently weighs 5.9k gzip.
  • Server rendering is important. We should pick something that makes sense and go with that.
  • Flexibility: allowing you to customize various parts.
  • Better documentation. There are lots of things I love about Marty's docs so I'd like to see all of the code documented (including addons) along with guides and plenty of examples.

@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:

  • Leaves alt and marty unchanged.
  • We can potentially backport features where alt and marty are just specialized builds (alt unopinionated and marty with more opinions?)
  • We can name the frankenflux: malt which is like a tasty beverage.

Cons:

  • Will probably hurt adoption. This is a big one.

Moving into Alt

Pros:

  • This repo has more stars and npm downloads (25k/month)
  • It already has a wide adoption.
  • It is not named after something from back to the future.

Cons:

  • Googling for alt or altjs kinda sucks.
  • We would be renaming the marty org.

Moving into Marty

Pros:

  • It already has a wide adoption.
  • martyjs is easier to search for.

Cons:

  • The alt repo and npm name would be abandoned.
  • We are now named after this dude.

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

  • Take Marty's instances.
  • Take Marty's class extends Store approach (better for typing and tooling).
  • Take Marty's HoC for connecting stores.
  • Merge Queries and Sources into just DataSources.
  • Keep constants but hide them away optionally.
@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

@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.

Collaborator

taion commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@justjake

justjake Jun 18, 2015

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:

  • submit async requests via action creators
  • API also fires a startGet / startUpdate (or whatever you want to call it) action when you submit a request
  • store or proxyStore listens for startUpdate and queues the update internally, allowing easy rollback if the request fails
  • provide request-progress-state information along with business data when view components or other stores retrieve data.

justjake commented Jun 18, 2015

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:

  • submit async requests via action creators
  • API also fires a startGet / startUpdate (or whatever you want to call it) action when you submit a request
  • store or proxyStore listens for startUpdate and queues the update internally, allowing easy rollback if the request fails
  • provide request-progress-state information along with business data when view components or other stores retrieve data.
@Shearerbeard

This comment has been minimized.

Show comment
Hide comment
@Shearerbeard

Shearerbeard Jun 18, 2015

@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.

Shearerbeard commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

@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.

Collaborator

taion commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 18, 2015

Collaborator

@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.

Collaborator

taion commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@justjake

justjake Jun 18, 2015

@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?

justjake commented Jun 18, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 20, 2015

Owner

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.

Owner

goatslacker commented Jun 20, 2015

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

This comment has been minimized.

Show comment
Hide comment
@Shearerbeard

Shearerbeard Jun 20, 2015

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.

Shearerbeard commented Jun 20, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 20, 2015

Collaborator

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:

  • Get rid of explicit constants and just use constants from actions (hopefully there are no poor benighted souls out there that are just passing around raw strings as constants)
  • Move to converged fetch/query/data source implementation

Alt users:

  • Use HOC method for containers instead of AltContainer
  • Move to class extends Class approach
  • Move to converged fetch/query/data source implementation

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.

Collaborator

taion commented Jun 20, 2015

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:

  • Get rid of explicit constants and just use constants from actions (hopefully there are no poor benighted souls out there that are just passing around raw strings as constants)
  • Move to converged fetch/query/data source implementation

Alt users:

  • Use HOC method for containers instead of AltContainer
  • Move to class extends Class approach
  • Move to converged fetch/query/data source implementation

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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 21, 2015

Owner

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.

Owner

goatslacker commented Jun 21, 2015

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

This comment has been minimized.

Show comment
Hide comment
@jdlehman

jdlehman Jun 21, 2015

Collaborator

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.

Collaborator

jdlehman commented Jun 21, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 21, 2015

Collaborator

@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?

Collaborator

taion commented Jun 21, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 21, 2015

Owner

Ok cool, so plan of action...

  • Nail down what we're merging exactly.
  • Figure out a merge path.
  • Create an org where all the addons can live.

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?

Owner

goatslacker commented Jun 21, 2015

Ok cool, so plan of action...

  • Nail down what we're merging exactly.
  • Figure out a merge path.
  • Create an org where all the addons can live.

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 21, 2015

Collaborator

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 #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.

Collaborator

taion commented Jun 21, 2015

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 #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

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Jun 21, 2015

Collaborator

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

  • HoC
  • Stores
  • Action Creators
  • Application (Or whatever we use)

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

  • Which approach should we adopt?
  • Whichever approach we choose, how do we move provide a migration path for it?

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

Collaborator

jhollingworth commented Jun 21, 2015

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

  • HoC
  • Stores
  • Action Creators
  • Application (Or whatever we use)

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

  • Which approach should we adopt?
  • Whichever approach we choose, how do we move provide a migration path for it?

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 21, 2015

Collaborator

@jhollingworth

#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...

Collaborator

taion commented Jun 21, 2015

@jhollingworth

#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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 21, 2015

Collaborator

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.

Collaborator

taion commented Jun 21, 2015

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

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Jun 22, 2015

Collaborator

#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 :)

Collaborator

jhollingworth commented Jun 22, 2015

#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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 22, 2015

Collaborator

@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.

Collaborator

taion commented Jun 22, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 22, 2015

Collaborator

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.

Collaborator

taion commented Jun 22, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 26, 2015

Collaborator

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.
Collaborator

taion commented Jun 26, 2015

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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 26, 2015

Owner

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

Owner

goatslacker commented Jun 26, 2015

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

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 26, 2015

Collaborator

@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?

Collaborator

taion commented Jun 26, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@goatslacker

goatslacker Jun 26, 2015

Owner

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.

Owner

goatslacker commented Jun 26, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 27, 2015

Collaborator

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

Collaborator

taion commented Jun 27, 2015

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

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jun 27, 2015

Collaborator

In checklist form:

Collaborator

taion commented Jun 27, 2015

In checklist form:

@jhollingworth

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Jul 31, 2015

Collaborator

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!

Collaborator

jhollingworth commented Jul 31, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Jul 31, 2015

Collaborator

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.

Collaborator

taion commented Jul 31, 2015

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

This comment has been minimized.

Show comment
Hide comment
@ariddell

ariddell Sep 4, 2015

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?

ariddell commented Sep 4, 2015

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Sep 4, 2015

Collaborator

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

Collaborator

taion commented Sep 4, 2015

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

@ariddell

This comment has been minimized.

Show comment
Hide comment
@ariddell

ariddell Sep 4, 2015

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:
#337 (comment)

ariddell commented Sep 4, 2015

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:
#337 (comment)

@jhollingworth

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Sep 4, 2015

Collaborator

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:
#337 (comment)

Reply to this email directly or view it on GitHub.

Collaborator

jhollingworth commented Sep 4, 2015

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:
#337 (comment)

Reply to this email directly or view it on GitHub.

@dcousineau

This comment has been minimized.

Show comment
Hide comment
@dcousineau

dcousineau Sep 4, 2015

@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...

dcousineau commented Sep 4, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@pbomb

pbomb Sep 11, 2015

@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.

pbomb commented Sep 11, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@bishtawi

bishtawi Sep 17, 2015

@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.

bishtawi commented Sep 17, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@dcousineau

dcousineau Sep 17, 2015

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

dcousineau commented Sep 17, 2015

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

This comment has been minimized.

Show comment
Hide comment
@jhollingworth

jhollingworth Sep 17, 2015

Collaborator

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

Collaborator

jhollingworth commented Sep 17, 2015

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment