Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving Onionify to this repo #620

Closed
staltz opened this issue May 30, 2017 · 13 comments

Comments

Projects
None yet
10 participants
@staltz
Copy link
Member

commented May 30, 2017

As onionify is getting more mature, more people are starting to use it, and its API is taking inspiration from Cycle Collections, we could consider essentially merging Cycle Onionify with Cycle Collections and placing it in this repo as an official package.

That said, before any of that, here are two questions to be answered:

  • What name should we give to it? ➡️ @cycle/state got good votes
    • @cycle/onionify: 👍 familiar, 👎 a bit hard to type, to remember, and not obvious what is its purpose from the name
    • @cycle/onion: 👍 a bit familiar, 👎 still not obvious what is its purpose from the name
    • @cycle/state: 👍 obvious purpose from the name, "onion" was just a reference to its fractal qualities but you could use the library without its fractal qualities, the name is simple and straight-forward like other official packages, 👎 some people might still like "onionify"
    • something else?
  • Are there any use cases that Cycle Collection solves which Cycle Onionify does not solve through the new collection API?
  • Decide the new Callbag-based API

I remember @Widdershin preferred to keep both Onionify and Collections as two options where users pick what they feel is best. Does that stance remain, now that we have the two things blended? Is it too early to make it an official package? (Note that people have been wishing for docs on state management on the website. Either we document only one official state management solution or we document none, or document all of them.)

Note that I'm putting this issue here but this doesn't mean a package will be soon made official, it's just up for discussion at this point.

@TylorS

This comment has been minimized.

Copy link
Member

commented May 30, 2017

I'm not super up-to-date on onionify or collections, and have used neither, but for naming maybe @cycle/reduce or @cycle/lenses

@abaco

This comment has been minimized.

Copy link
Contributor

commented May 30, 2017

I like @cycle/reduce! It's clear enough, given how popular is Redux, and it captures the essence of the library. Lenses are not essential, I wouldn't name the library after them.

@staltz

This comment has been minimized.

Copy link
Member Author

commented May 30, 2017

@abaco It's not my favorite name, in general I don't like the word "reduce" unless you get an output smaller than the input, which is the case for Array reduce, but not for Stream reduce. (In fact, in Haskell this is more consistent, the correct name would be scan like RxJS has)

But I think it's still a valid option to consider, since anyway we are using the name "reducer" as a building block in onionify.

@staltz

This comment has been minimized.

Copy link
Member Author

commented May 30, 2017

Another thing to take into consider with the name is that it'll be used as the channel name too. @cycle/foo would imply sources.foo and sinks.foo (just as a convention of course, since you could give any name you want to the channel). I don't find sources.reduce to be helpful to indicate its purpose. On the other hand sources.state.state$ might look weird, if we choose the name @cycle/state we should consider renaming StateSource.state$.

@jvanbruegge

This comment has been minimized.

Copy link
Member

commented May 30, 2017

I like @cycle/state
and 👍 for moving it to the main repo once collection is merged

@Widdershin

This comment has been minimized.

Copy link
Member

commented May 31, 2017

I remember @Widdershin preferred to keep both Onionify and Collections as two options where users pick what they feel is best. Does that stance remain, now that we have the two things blended?

I feel that onionify and collection address different problems. In past I've said that I view onionify's handling of dynamic lists as being one of its biggest weaknesses.

I very much like the idea of blending them together, and in my mind this would address one of the biggest weaknesses in onionify. A standalone collection library could be useful, but the current collection codebase needs much love.

I also recently read about a philosophy for creating libraries where you create simple, composable primitives as the base of the library. You then create a prebuilt component that addresses 80% of the common use case.

collection was originally a simple primitive one would fold over. We then changed it to be the prebuilt component. I want to see both of these well balanced in a standalone collection library, to allow users simple use and flexibility for more complex problems.

So here's what I would like to see happen:

  • onionify meets collection! there is much rejoicing
  • onionify continues to have its own implementation of collection
  • we rewrite standalone collection with the lessons from the onionify implementation
  • onionify can then depend on the nice and shiny standalone library

This road also allows us to enable ongoing innovation instead of putting all of our onions in one basket. Other libraries might spring up and choose to also depend on the standalone collections library.

tl;dr: 😸👍

@FeliciousX

This comment has been minimized.

Copy link
Contributor

commented May 31, 2017

I like @Widdershin 's idea.

Honestly I've been waiting for the moment where collection and onionify merges.
There is much to rejoice :D

I am okay with @cycle/state

@whitecolor

This comment has been minimized.

Copy link
Contributor

commented May 31, 2017

garlicify

@kylecordes

This comment has been minimized.

Copy link

commented Jun 2, 2017

@cycle/state seems like a very fine name for a library intended as the typical default way of managing state. In explaining bits of code to others I have found the word onion is a distraction even though it makes sense relative to the diagram explaining it in the concept behind it.

@staltz

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2017

@Widdershin this issue has progressed now that we finally discovered the right API for onionify+collection, and it's surprisingly similar to cycle/collection:

in cycle/collection:

in cycle-onionify:

  • makeCollection(options)

About your plan to go forward:

  • onionify meets collection! there is much rejoicing
  • onionify continues to have its own implementation of collection
  • we rewrite standalone collection with the lessons from the onionify implementation
  • onionify can then depend on the nice and shiny standalone library

Step 1 and 2 are done. What puzzles me is how to do step 3 and 4 without recreating onionify, because cycle/collection uses reducers and folding: https://github.com/cyclejs/collection/blob/15f80a15de47d9b64985a37d40d799d903d702b1/src/collection.js#L72-L77

I'd like to imagine we could just pick the Collection.ts file from onionify and publish that as a separate package, but it turns out it assumes a lot of onionify infrastructures to be in place, e.g.:

If we were to untangle collection and onionify so that collection can be its own package, we'd have to figure out how onionify can inject those two assumptions above into makeCollection.

It doesn't seem that impossible, it's just a bit tricky. Perhaps @jvanbruegge @abaco @ntilwalli could help with some ideas here.

That said, a more important question to ask before solving this technical aspect is: what use cases do we see where people would use the underlying collection package without using onionify?

@geovanisouza92

This comment has been minimized.

Copy link

commented Oct 3, 2017

We assume the array$ comes from sources.onion.state$:

  const List = makeCollection({
    item: Child,
    itemKey: (childState, index) => String(index),
    itemScope: key => key,
    collectSinks: instances => {
      return {
        onion: instances.pickMerge('onion'),
        // ...
      }
    }
  });

When I was looking at the API some days ago, it seemed weird that I have to provide itemKey and itemScope, that could have smart defaults (itemKey could be data.key || index, and itemScope defaulting to the itemKey, e.g.), while the actual data be assumed to be a onion state.

So, how would be the effort to separate it from onionify, maybe by using a common interface (set of requirements about data structure and child component sources interface)?

@staltz

This comment has been minimized.

Copy link
Member Author

commented Mar 9, 2018

Update: it seems that now with #760 and a Callbag-driven architecture, we can solve the naming problem sources.state.state$ so that sources.state is a Callbag. This will add support for streaming new state updates (like observables or xstream do), but also allows remembering the latest state object and sending it when the state callbag is pulled.

@VasilioRuzanni

This comment has been minimized.

Copy link

commented Mar 25, 2018

@staltz Is the main side of a Cycle app going to be callbags all the way then? (which makes sense given we need both push and pull). The actual question is what's going to be a strategy for converting to/from most/xstream/RxJS observables? Is that going to be just explicit fromObs callbag usage on the main side or something less explicit like current adapt(...) calls behind the scenes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.