@zaceno There's a little issue when using emit inside the ready event. Since at that time app hasn't yet returned, emit is not available.

Codecov Report

Merging #280 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #280   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         143    143           
  Branches       44     44           
=====================================
  Hits          143    143

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7b2c16...a891113. Read the comment docs.

Stupid question but why ?

It is all about purity, but, if I dont want to use this pattern : (Yeah I know emit/event is not pure by meaning)

const emit = app({
  events: ...
  actions: ...
})

but directly

app({
  events: ...
  actions: ...
})

I wont be able to emit events ?

If I declare events in a file and actions in another one, where emit will come from ? Circular dependencies ?

@Swizz the solution would be to define your actions and events in one or more mixins. Since mixins get emit passed to them.

So there is a way. Wether you like that way or not, though... I can see how this might not be liked by everybody. (I like it though)

For me: getting emit passed to mixins is the best thing about this. Cleaning up the api by removing the emit parameter to actions and events would be nice, but if people are generally against it it may not be such a good idea.

I am not fundamentally against (not angry here), I was just raising questions in the case, in a genius bloom, nobody was thinking about it

@Swizz It is a good question to raise. Because it does mean that if you intend to compose together your app-options, and you need to emit custom events, you must use mixins. (Or do something really hacky. Not sure how, but there's probably a way)

I don't personally know of any valid reasons why you wouldn't compose your app using mixins. But maybe such reasons exist

I think this is okay in preparation of a 1.0 release. If the world falls apart later, we can just add it again and it would not break anything, because:

From

import { app, h } from 'hyperapp'

const state = {
  count: 0
}

const actions = {
  inc: (state) => ({ count: state.count + 1 }),
  done: (_state, _actions, _data, emit) => emit('done')
}

const view = (state, actions) => {
  return h('button', { onclick: actions.inc }, state.count)
}

const emit = app({ root: document.getElementById('app'), state, view, actions })

to

import { app, h } from 'hyperapp'

const state =
  count: 0
}

const actions = (app, emit) => ({
  actions : {
    inc: (state) => ({ count: state.count + 1 }),
    done: () => emit('done')
  }
})

const view = (state, actions) => {
  return h('button', { onclick: actions.inc }, state.count)
}

const emit = app({ root: document.getElementById('app'), state, view, mixins: [actions] })

@Swizz emit is considered a lower level function. You rarely need it in your app. Why would you trigger an event if you can call an action? Now, when you do need it, it's probably better to hide that complexity in a mixin.

One of the primary use-cases I've found for emit, is actually when I'm trying to decouple my app into separate mixins.

Where sometimes in mixin A, I'll need to call an action in mixin B. Now I may know the path to the action in mixin B. But it's not good practice to hardcode that sort of logic into mixin B. I'd like to be able to change mixin A without breaking mixin B.

So I'll use events for that. Rather than call the action in mixin B, I'll just emit an event, and have mixin A listen for it.

I will need to change my mind about events tho.
In a Vuex way, I was using actions as Mutators (things that change the state or call events) and events as Actions (things will do not pure stuffs).

By the way, about passing the app into mixins. Other than the view is useless information because the state is the global state before other mixins have loaded, the actions are not yet initialized/wrapped.

@jbucaran I was just looking at that yesterday. Why do we do that? What's the envisioned use case? (Maybe that's for a slack-discussion, but anyway)

@Swizz If you were using events in that way and this PR causes you to realize that was not the hyperapp-way, then I think we succeeded. While you can continue to do so at a certain extent with mixins, you should be using actions both as a reducers/mutators and side effects.

@zaceno Why do we do that?

Because I didn't know what else to pass. Shall we knock it off too?

@Swizz: back in the day we used to have "reducers" = vuex mutators, and "effects" = vuex actions. Then we merged them together into hyperapp's actions which mostly resemble vuex mutators, but are able to call other actions, and do async stuff, so a little like vuex effects at the same time.

@jbucaran: why not, if there's no reason for it. It would certainly help the sinister alchemy I've got cooking.

@Swizz With regards to that reason, it was a tradeoff (to me). By making actions both mutators/effects we were not really changing how anything worked internally, but it made the API much cleaner and easier to learn.

@zaceno I've updated this PR so we don't pass the app to mixins either.

I also checked all the mixins I know in existence, ones I've made and ones that have been published as I was made aware of and none were using appOptions. Actually, the only one was the router, which I'll make sure to update after this is merged.

@matejmazur @jamen @andyrj have all authored mixins, so this PR is of your interest. Please have a look when you have a chance. 🙏

I don't believe I utilized emit in my mixins, so as long as actions and events are still going to be available inside a mixin I am ok with this change and can update how actions are initialized. I like the cleaner api this provides.

These changes seem okay to me. I haven't personally used appOptions or emitin my mixins yet, so it doesn't affect me. But, I wasn't really a fan of appOptions to begin with.

I'm ok with it.

Forcing people to use mixins when they want custom events seemed like a strange idea at first, but I'm warming up to it. Restricting how people use parts of the framework is certainly in line with the Elm philosophy.

It is nice that you can emit events directly from the view functions with this change.

The only downside is the way to use emit at the main app-level actions now is really awkward in my opinion. It almost looks like it shouldn't work 😆

const emit = app({
  actions: {
    alldone:  () => emit('done')
  }
})

@SkaterDad It will work in actions because they will run after the app() call has returned for sure.

It will not work for events.ready though, since that one runs before app returns.

🎉