Hyperapp: [RFC] Components & Mixin Guidelines

[jorgebucaran]

Mixins and components are not the same thing!

We use components to describe parts of our app, like a TabBar, a ToggleButton, SlideButton, etc., and mixins to extend the state, actions and events of our app.

_To extend_ means that we will merge your mixins state/actions/events with your app state/actions/events to create a single state/actions/events.

But often you need to pass actions to your component via props.

CodePen

function ToggleButton({ toggle, isOn }) {
  return ( < /* ... */ > )
}

app({
  actions: {
    toggle(state) {
      return { isOn: !state.isOn }
    }
  },
  view(state, actions) {
    return <ToggleButton toggle={actions.toggle} isOn={state.isOn} />
  }
})

This makes components, in a way, really "hollow" as we must teach it pretty much everything so it can thrive in the world. This is good, as it makes components extremely easy to test and debug. But what if the implementation of those actions is provided by a mixin?

In that case I think the mixin and component should be tucked away into their own module.

A good example of this is our friendly Router's Link component.

import { router, Link } from "@hyperapp/router"

app({
  view: [
    [
      "/",
      (state, actions) =>
        <Link to="/test" go={actions.router.go}>
          Test
        </Link>
    ],
    [
      "/test",
      (state, actions) =>
        <Link to="/" go={actions.router.go}>
          Back
        </Link>
    ]
  ],
  mixins: [router()]
})

Notice how we must teach Link actions.router.go, which is fed to our actions by the router() mixin. I mean, it's not so bad right? Just a little repetition is not so bad and this makes the Link component easy to test and debug and all that, right?

Sure! but the Link component would be just as easy to test and debug and all that if it shipped with actions.router.go already "pre-wired"... as long as you can overwrite the action and pass it a new one, just like it's done above.

There is a PR here addressing just this. The idea is to be able to rewrite the example above as follows:

import { h, app } from "hyperapp"
import { router } from "@hyperapp/router"

app({
  view: (state, actions, { Route }) => <Route />,
  mixins: [
    router([
      [
        "/", (state, actions, { Link }) =>
          <Link to="/test">
            Test
          </Link>
      ],
      [
        "/test", (state, actions, { Link }) =>
          <Link to="/">
            Back
          </Link>
      ]
    ])
  ]
})

Notice now we don't need to teach the Link actions.router.go anymore! 💯

On the other hand, see that we now need to grab the pre-wired Link component somehow, since:

import { Link } from "@hyperapp/router"

...would take us back to square one (wouldn't work unless we pass actions.router.go ourselves).

The benefit is less boilerplate, but it comes at a cost: we need to overload the view function with a third argument! 🤔

The good news is that none of this requires changes in core! 🎉

This is already possible using events.render.

But the real question to me is which way is more expensive? Overloading the view function or having to manually wire components every time we use them?

The only caveat about overloading the view function is you need to make sure not to break other authors also overloading the function. This is already solved, of course, using "namespaces", which fortunately we already support.

actions: {
  myNamespace: {
  // My actions
  },
  yourNamespace: {
  // Your actions
  },
  ...
}

Now, while the Router and Link component are good examples to introduce this idea, we could easily shrug it off and get away without it just as we have done until now.

IMO more complex components will benefit from this pattern. Imagine a rich text editor in Hyperapp.

It could look as follows:

import { h, app } from "hyperapp"
import { editor } from "./editor"

app({
  view(state, actions, { editor: Editor }) {
    return <Editor />
  },
  mixins: [editor({/*...*/})]
})

Compare how it might look like without overloading the view function:

import { h, app } from "hyperapp"
import { Editor, editor } from "hyperapp-rt-editor"

app({
  view: (state, actions) => (
      <Editor state={state.editor} actions={actions.editor} />
  ),
  mixins: [editor({/* ... */})]
})

Not the worst, but I prefer the previous pattern. What about you?

Whichever style you end up choosing for your own components+mixins is ultimately up to you.

:wave:

[zaceno]

👍 for view(state, actions, {editor: Editor}) {...}

[lukejacksonn]

Much prefer <Editor /> over <Editor state={state.editor} actions={actions.editor} />

[andrewfluck]

I'm all for view(state, actions, { mixin: Component })

[zaceno]

All who have answered and/or reacted seem unanimous 😄

Perhaps we can go ahead and merge https://github.com/hyperapp/router/pull/20 then?

[jorgebucaran]

@zaceno Yes, let's give it a few more days though! :)

/cc @andyrj @ngryman @jamen @SkaterDad

[SkaterDad]

Like the other comments so far, I'm highly in favor of this pattern. It's a really elegant solution, which in my opinion fits the way hyperapp already deals with actions, state, etc.

[jamen]

I love the direction this has gone. Probably will start using that personally. I was trying to go for something like that back in https://github.com/hyperapp/hyperapp/issues/235, but the idea had major drawbacks. This is a great! :smile:

[andyrj]

I agree with @zaceno and @lukejacksonn. The overloaded view with third param is cleaner.

[infinnie]

Maybe a good old jQuery event handler would solve the aforementioned problem:

app({
    // ...
    view: function () {
        return <a href="/test" data-route-link="true">Go</a>;
    },
    root: $("#root").get(0),
    mixins: [router()],
    events: {
        load: function (state, actions) {
            $("#root").on("click", "[data-route-link=true]", function () {
                actions.router.go($(this).prop("href"));
                return false;
            });
        }
    }
});

[jorgebucaran]

@inf This is off-topic, but just a heads-up, you can rewrite that load event as follows:

...
    events: {
        load(state, actions) {
            $("#root").on("click", "[data-route-link=true]", function () {
                actions.route.go($(this).prop("href"));
                return false;
            });
        }
    }

[okwolf]

@jbucaran the inconvenient 800-pound gorilla in the room is that this amounts to a single-level form of dependency injection that trades one problem for another. In a more realistic example you would probably have separate components for the views of each route instead of inlining the entire view. Then you would still need a way to pass the pre-wired™ Link component to each of your components for different routes. Unfortunately the current calls to component functions only take data and children so today you would have to pass the Link component as data (or props as they are called in React and Vue.js).

React provides context as their solution to this exact problem (it's even used by React Router) but I always found context to be hacky and it comes with severe discouragement against use from the creators. I wish I had a proposal for how we can resolve this part of the issue. Maybe after some more thought... 🤔

[SkaterDad]

@okwolf It certainly is dependency injection, but it's no different than how actions already work, in my opinion. I agree it probably has downsides, though.

I don't use JSX, so my "higher level" view functions usually have the signature function MyView(state, actions) { .... }. I definitely see your point about how this extra "views/widgets" parameter would be somewhat inconvenient in JSX. However, I think it's becoming more popular in React to pass in view functions to a component, so... it's not _too_ weird? downshift is a recent example.

Another potential downside is these injected components are harder to tree-shake/dead-code-eliminate? Not sure how the bundler or uglify would know if you're only using 2 of the 10 components you've injected from a mixin. For components I'm making myself, for my own apps, this is probably not a big deal, since I can just comment out what I'm not using. But for someone using a library of components using this method, it could be a more complicated problem.

I'm totally comfortable using this pattern for the router mixin, since the components are all necessary when using the router. I'm also totally cool with it for creating custom components I'll use throughout my app. Would I bring in a whole "material design" component library this way? Tougher decision.

Like every other discussion in programming, it's all about what trade-offs you prefer, I suppose 😄

[Mytrill]

I feel there is something I am missing here, but why not something like this?
Link.js

let defaultGo
export const setDefaultGo = (go) => {
  defaultGo = go
}

export function Link(props, children) {
  //...
  props.onclick = function(event) {
    //...
    event.preventDefault()

    if(props.go) {
      props.go(props.href)
    } else if(defaultGo) {
      defaultGo(props.href)
    }
  }

  return h("a", props, children)
}

router.js:

import { setDefaultGo } from './Link'

export function router(options) {
  return function(emit) {
    return {
      events: {
        load: function(state, actions) {
          // at this time the actions are already wired
          setDefaultGo(actions.router.go)
        }
      }
    }
  }
}

This particular implementation is quite dirty (and NOT tested) but something of this type should work no?

[zaceno]

@Mytrill the notion of exporting pre wired components rather than "injecting" them as discussed here, has been explored to some extent in this PR: https://github.com/hyperapp/router/pull/20 but was not as popular.

Your method has a technical problem though, which is: what if there are multiple apps on a page, and they all include your mixin? Which "go" should we use? Granted it's not likely a problem with your example, because when would anyone realistically use multiple routers in a single page :-P ... but still, for other mixins that are meant to be reusable, we can't rely on closures in module scope because that will be shared between apps that use the mixin.

[jacktuck]

To extend means that we will merge your mixins state/actions/events with your app state/actions/events to create a single state/actions/events.

If i'm not misunderstanding and mixins are just for extending state/methods, i'm more partial to higher-order components/functions. Mixins, to me, kinda feels like state is being changed from under you which is a bit of a code smell to me.

import { Editor, editor } from "hyperapp-rt-editor"

const App = app({
  view: (state, actions) => (
      <Editor state={state.editor} actions={actions.editor} />
  )
})

export default editor(App)

[jorgebucaran]

@jacktuck You won! We are not moving forward with this. 🎉

See: #219