thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 199 TypeScript projects (and ~180 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.38k stars 150 forks source link

Support context in hdom #13

Closed allforabit closed 6 years ago

allforabit commented 6 years ago

Maybe there's already a way of achieving this using the existing api or via native es6 methods but I am looking to pass values down deeply into the hdom tree. Thus far I have explored ideas of a higher order function to mimic behaviour found in the redux connect method. The idea of this is to wrap the component and pass in relevant props to the child. To do this, it uses context to pull out the store which has been injected higher up in the component tree (using a HOC called a provider). It's this second part that I'm hitting a stumbling block with.

From digging into the code I can see that the bulk of the work that is carried out to render the hiccup with hdom is done in the normalizeTree function. Would this be a good place to add the possibility to inject globally available state (well globally to the hdom tree).

I understand that an option would be to make the state available in an outer scope but I am hoping to avoid this so that everything is as self contained as possible in a similar way that redux components are.

I don't see a way of adding labels here, I'm wondering if I have access? Thanks!

postspectacular commented 6 years ago

Hey @allforabit , did you have a look at the login form example I posted last week? This shows the overall pattern of using a central state (like redux does too), but also shows how derived views attached to the state work within components. For brevity the example uses a few global vars, but this can be easily refactored to pass views as args to component functions instead (or have a component create its own derived view). This still keeps the state centralized, since views only act as readonly pointers to somewhere within the larger app state.

Let me know if that helps. If not I will make up another example...

postspectacular commented 6 years ago

Also, related here's some more info about using atoms and derived views: https://github.com/thi-ng/umbrella/tree/master/packages/atom#derived-views

allforabit commented 6 years ago

Yes I had a look at this and I can see how this can fulfil a similar role. I probably just need to change my conceptual frame of mind a little!

I wonder what the best way of achieving updates from components that are derived from read only views? I'm guessing it would be a matter of passing in callbacks to be handled by the ancestor component that has access to the atom/cursor. In the example the updater functions, that the components use, access the global db reference.

I know it's probably not the end of the world if a single global db is used and this is how reframe does it. This does have implications for tools like devcards though and makes it difficult to have nested apps. https://github.com/Day8/re-frame/issues/137

postspectacular commented 6 years ago

I think I see where you're going, but am not entirely sure what that "context" you're asking for would allow you to do (am not familiar w/ devcards). Can you please provide/describe a concrete example? How does this pattern work in redux? Also, unlike re-frame which defines the central state atom as singleton in the library/framework itself (at least it used to, haven't used in a few years...), hdom does not care about your app state at all, so it's not really comparable (IMO same goes for comparing w/ redux). I've kept state handling separate for exactly this reason: to be able to experiment with new/different approaches, rather than "complecting" all these tasks: state handling, event/action dispatch and DOM creation/diffing... AFAIK redux is based on the same approach as re-frame and both piggyback on other vdom implementations (React), whereas hdom only handles the vdom parts by design.

Therefore I think this issue boils down to how you get state and context information into your components when they're created and with that I think you listed the main options already: define your components as closures and pass any context info when creating them. Will mock up a new example for that tonight.

Btw. The two things I always loved in re-frame the most are the idea of event batching and its use of interceptors to augment event processing. There's another module in the works for that (just need to find time to refactor it first).

allforabit commented 6 years ago

A concrete example to provide an atom to a hdom tree and allow children of the hdom that are nested deeply in the tree to access this atom. So imagine a tree path such as app -> sidebar -> userProfile -> editUserProfile. To get a user cursor down here we'll need to either pass it down through the sidebar if we want it to flow down from the root component. Alternatively we can pull it out of the scope. What something like context in react does is allows you to avoid passing props right down through the tree and create a provider/consumer relationship between distant ancestor components. Redux is one example of it's usage with react. Others include for localization, theming and for routing (react router and styled components).

I hear you about keeping things separated out and the examples I've provided are more like frameworks. My point is more that react the library provides this extension point for frameworks to use and makes certain things a bit more convenient. A key difference between redux and reframe is that redux doesn't use a singleton and this achieved by using the react context api. As far as I can see it's the one downside of reframe vs redux. Yes I love the idea of interceptors (as well as there concept of sidefx and cofx). I can't wait to see what you come up with for that!

I'm wondering would a very opinionated low level method to allow some sort of preprocessing to take place on each of the components make sense? At the moment both the hdom start and normalizeTree take "span" as a boolean argument. I wonder could this be made more generic and be made into a function. As this function could be stateful it would make it possible to create a context like system that could create relationships between distantly related components.

I could try to put together a pull request for this if you think it's worth exploring? I also understand if you'd prefer not to go down this route as it has the potential of complicating/"complecting" things.

allforabit commented 6 years ago

Actually to clarify, I will put together an example using react's context and the umbrella atom. I will follow up with this in the next few days.

postspectacular commented 6 years ago

Hi again, since you referenced Bruce's Devcards earlier, I just added & uploaded a new example, hopefully showing some this (and some options) in a bit more detail (lots of comments in the source too).

https://github.com/thi-ng/umbrella/tree/master/examples/devcards

http://demo.thi.ng/umbrella/devcards/

Will respond to your other points tomorrow...

postspectacular commented 6 years ago

Just read through the React docs about context and this section looks potentially feasible to support as optional feature, but personally I still don't fully understand the point or benefit over passing these things manually as a more obvious/clear/readable solution to which data is ending up where. In the end you're still passing it manually anyhow by having to declare .contextTypes = {...}.

So far this all smells a little of "convenience magic" and it also encourages a kind of local state in components, which I always try to avoid (though am aware this local state is largely about component config data). But don't get me wrong, am not saying no. Looking forward to your example...

postspectacular commented 6 years ago

Btw. I also like this quote on the React docs page and is fundamentally what I proposed above as more natural solution:

Before you build components with an API similar to this, consider if there are cleaner alternatives. For example, you can pass entire React components as props if you’d like to.

This pattern is also demonstrated in the above mentioned devcards demo...

allforabit commented 6 years ago

Thank you very much for that demo. It's amazing how much functionality and interactivity that can be expressed in such a small amount of code! Yes I can see your point about keeping things clearer by manually passing down data. Having something like context somewhat moves it away from being a strict tree of data that is easy to reason about.

Have you seen this project? https://github.com/roman01la/citrus/blob/master/README.md It's an evolution of some of the ideas from re-frame and has support for batched update, side effects, efficient subscriptions and the whole shebang. It avoids using the singleton db by passing a "reconciler" object through the tree. The reconciler is similar to a redux store. He mentions that it is possible to inject this into the tree by using react's context but advises against it:

Passing reconciler explicitly is annoying and makes components impossible to reuse since they depend on reconciler. Can I use DI via React context to avoid this?

Yes, you can. But keep in mind that there's nothing more straightforward and simpler to understand than data passed as arguments explicitly. The argument on reusability is simply not true. If you think about it, reusable components are always leaf nodes in UI tree and everything above them is application specific UI. Those leaf components doesn't need to know about reconciler, they should provide an API which should be used by application specific components that depend on reconciler and pass in data and callbacks that interact with reconciler.

So I'm not even convinced myself if using context is the best approach, particularly since hdom aims to be much simpler! It might be nice to have it as an advanced feature that in general shouldn't be used directly but by libraries or frameworks that are using hdom (similarly to react).

Just to note, I'm going to use the latest alpha version of react (16.3) for my demo. This has a more refined context system and will be a public api when it's released. Details of this are here: https://github.com/reactjs/rfcs/blob/master/text/0002-new-version-of-context.md

postspectacular commented 6 years ago

Okay, a long night made worth it, i hope... :) Just pushed @thi.ng/atom 0.9.0 and new demo showcasing my existing event & interceptor handling, which has been used v. successfully for a bunch of apps already in production... (having said am sure there's ample scope for further refactoring, always! :) )

http://demo.thi.ng/umbrella/interceptor-basics/

To make more sense of the demo, the important bits are here (in the comments):

https://github.com/thi-ng/umbrella/blob/master/examples/interceptor-basics/src/index.ts https://github.com/thi-ng/umbrella/blob/master/packages/atom/src/interceptors.ts https://github.com/thi-ng/umbrella/blob/master/packages/atom/src/event-bus.ts

(I will also add more detailed docs about these new features over the coming days, at the moment a lot of the comment assume familiarity with the overall concept of interceptors)

allforabit commented 6 years ago

This is amazing, thanks so much for pushing it live (and for the rest of this treasure box of libraries!) I hope you're not too tired today!!! It's like Christmas morning every morning, seeing all the new additions :)

I will use this event bus for the react integration that I'm working on (which I will post here asap). This more or less clarifies how you'd go about tying the various pieces of the library together. Feel free to close this issue as I think it's unneeded and can be addressed by passing around the event bus as a prop similarly to how it's recommended with Citrus.

postspectacular commented 6 years ago

Thanks for the kind words! I too think (objectively) it's pretty workable, but leaving this open for a while since there might a few more "hiccups" (like #14) popping up. The library version of atom is somewhat different (and in many ways more minimal/focused) than earlier versions I've used for other projects, but there's always room for improvement...

allforabit commented 6 years ago

I've got a demo of the new React context api in action with the Umbrella libraries managing more or less everything except for the rendering. https://github.com/allforabit/umbrella/tree/react-context-example/examples/react-context, demo here: https://allforabit.com/react-context/

It's an autocomplete widget that searches Wikipedia (inspired by an Om next demo). It's a little rough around the edges and I ended up using normal js/jsx instead of typescript because the new React apis aren't available on Definitely Typed just yet. To illustrate the independence of the connected components I've place them in a "dumb" layout component that just calls the connected components (without any arguments/props). The context system makes sure that the "smart" connected components receive the state that they need as well as the dispatch function from the central bus.

I can see that you've put together a very comprehensive demo of your approach to app structure and this is very helpful and instructive. I like the way a lot of the wiring between the different components and events is setup in the config and there's only one entry point through the app object. I think the big difference in the react approach is that everything is a component, whether it's a router or a state management tool. There's probably trade offs for both approaches but for now I will use the example you gave as a template for new projects.

Slightly separately, the react lib that is included with the demo (https://github.com/allforabit/umbrella/blob/react-context-example/examples/react-context/src/lib.jsx) is the beginning of a reasonably comprehensive state management solution for React (thanks to Umbrella!) Do you think this could be a good addition to the libraries or will I work on it in a separate repo? I would like to get the hiccup syntax working with react too.

postspectacular commented 6 years ago

Thanks for the effort, Kevin! This is really great and the comparison really helps - or rather - it will help me to think about this some more and then also can give you a proper answer to your other question. Alas no capacity to fully check this in detail over the next few days :( Just one more thing: there's no reason why the router (or the app itself) could not be packaged up as components too. Essentially the little derived view function defined here is all what's needed to turn the router into a component. I just personally prefer the to have functionality (here "routing", but also more generally) available in standalone form (without assuming it will be used with components). Though maybe offering a component wrapper is a good idea... Many roads lead to Rome! Finally, yesterday I started extracting core pieces from that latest demo into a new repo to be used as re-usable app skeleton, possibly with a little config "wizard"... anyhow, thanks again & more real soon!

allforabit commented 6 years ago

Thanks very much for that, yes the derived view approach is very flexible and I'm using that in the a react app that I'm working on now to integrate the thi.ng router. It works almost the same as your example except that the state views return jsx instead of hiccup. In general the pattern looks really nice to work with in that it is very transparent as to how the components, events, state and router all interact with each other.

Looking forward to the skeleton app. I'm not sure if this is relevant but the create-react-app project could be a good model to go with. When an npm package is called create-[something-here], yarn can run "yarn create [something-here] my-app". E.g. "yarn create react-app my-app". https://yarnpkg.com/lang/en/docs/cli/create/ The main thing the generated skeleton app does is add a dependency to a "react-scripts" package that has all the latest dependencies as well as the webpack config.

postspectacular commented 6 years ago

Hey, sorry still had no chance to look through your context demo (on weekend!) - but since I needed it urgently I've pushed an initial version of the project generator: https://github.com/thi-ng/create-hdom-app.

That yarn create approach was exactly what I intended, but in the end I didn't base it on create-react-app (only used a tiny part from it) since that seemed way too OTT with its dozens of options/extensions. That new skeleton app is v. close to the router-basics example from a few days ago, but has some small additions and also is configurable (there're actually 4 project templates).

allforabit commented 6 years ago

Thanks no rush with that! To be honest there's not that much in it and I think you've got the gist of it based on the thread. I think it's very low priority anyway.

yarn create works a treat! One very small thing is I had to do a yarn upgrade --latest to upgrade to get the latest version of the atom package to get a new feature that it depends on (forward side fx). Really nifty though apart from that.

postspectacular commented 6 years ago

Hi @allforabit - so sorry for the long pause on this front, but lots of other things (more pressing things) to sort out meanwhile. I've been thinking about a simple solution to this, which 1) makes the use of context more or less optional, 2) isn't too intrusive inside hdom and 3) doesn't break existing components (not sure yet, but also would be v. easy to fix/update). So I will play around with this preliminary plan over the weekend:

For example:

// ctx will be injected as first arg to all component functions
const foo = (ctx: any, body: string) => ["h1", ctx.foo, body];

// ctx will be injected as first arg to all lifecycle methods
const bar = {
  init: (ctx: any, el: Element, ...args: any[]) => { ... },
  render: (ctx: any, ...args: any[]) =>
    // context passed indirectly (wrap children in `[...]`)
    ["section", ctx.bar, ...args.map((x) => [foo, x])]
    // or alternatively w/ ctx passed directly
    // ["section", ctx.bar, ...args.map((x) => foo(ctx, x))]
};

start(
  "app",
  // root component fn
  () => [bar, "hello", "world"],
  // arbitrary context object
  // here just using element attribs
  // but could store event bus, app db views etc.
  {
    foo: {class: "f-headline lh-solid"},
    bar: {class: "bg-light-gray"},
    etc: ...
  }
);

Thoughts?

allforabit commented 6 years ago

Thanks Karsten, yes this seems like a good way of adding it. Although I initially had envisaged as an additional argument at the end. I'm sure you have good reasons to make it the first argument though. This looks like it'll add some convenience and avoid some unnecessary property passing, hopefully without compromising the simpler structure. Looking forward to seeing what you come up with. I will do another version of the autocomplete demo with it to test it out and see how it compares with the react api.

postspectacular commented 6 years ago

I also first thought about putting at the end so that it's completely optional, but it screws with the many use cases where a component function takes rest arguments and there having the context at the end makes it weird. I think as long as it's clearly explained that the first arg to a component will always be the context, it's easy to ignore it and update existing components (of which there shouldn't be many yet outside my own code base) :) I will push a feature branch with the changes later tonight...

allforabit commented 6 years ago

Thanks very much for adding this. From looking at the updated router-basics it looks very straightforward to use. I will take it for a spin over the next few days!

postspectacular commented 6 years ago

Hi Kevin, I just pushed v3.0.0 & thanks a lot for the idea & input! adding you to contributors list... :)

allforabit commented 6 years ago

Aw shuck thanks, glad that I could be of service!