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

Fix mixin init order. Close #329. #332

Merged
merged 1 commit into from Aug 18, 2017
Merged

Fix mixin init order. Close #329. #332

merged 1 commit into from Aug 18, 2017

Conversation

jorgebucaran
Copy link
Owner

  • Shave off presets (mixin mixins) in the process.
  • Update tests & docs.

- Shave off presets (mixin mixins) in the process.
- Update tests & docs.
@codecov
Copy link

codecov bot commented Aug 18, 2017

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #332   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         146    145    -1     
  Branches       45     45           
=====================================
- Hits          146    145    -1
Impacted Files Coverage Δ
src/app.js 100% <100%> (ø) ⬆️

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 4900c27...420282d. Read the comment docs.

@Swizz
Copy link
Contributor

Swizz commented Aug 18, 2017

This will change the way events are reduced by mixin too ?

In the case I am using a mixin PromiseResolve to resolve each action using a promise to a thunk call.

App:actions:fetch -> PromiseResolve:events:resolve -> App:events:resolve

@jorgebucaran
Copy link
Owner Author

@Swizz This will change the way events are reduced by mixin too ?

Before the order started with app, now app is last.

With this PR.

const A = () => ({
  events: {
    anyEvent(state, actions, data) {
      console.log(data) //=> Data:
      return data + "A"
    }
  }
})

const B = () => ({
  events: {
    anyEvent(state, actions, data) {
      console.log(data) //=> Data:A
      return data + "B"
    }
  }
})

const C = () => ({
  events: {
    anyEvent(state, actions, data) {
      console.log(data) //=> Data:AB
      return data + "C"
    }
  }
})

const emit = app({
  events: {
    anyEvent(state, actions, data) {
      console.log(data) //=> Data: ABC
    }
  },
  mixins: [A, B, C]
})

emit("anyEvent", "Data: ")

@lukejacksonn
Copy link
Contributor

LGTM :shipit:

@jorgebucaran jorgebucaran merged commit 0953b5b into master Aug 18, 2017
@jorgebucaran jorgebucaran deleted the fix/mixin-order branch August 18, 2017 14:15
jorgebucaran added a commit that referenced this pull request Jan 7, 2018
- Shave off presets (mixin mixins) in the process.
- Update tests & docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants