-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Version 1.0, huge refactor and re-write of pieces #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ound. I kind of like it.
…mizations for app.js
…er documentation.
…onstrate a concrete idea.
…her than a message/update.
…k to the core effects.
Breaking refactor and re-implementation of effects.
…bumps test covreage to 100%.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
This PR represents the changes needed to align better with TEA (the elm architecture), and to be usable in real world applications. This also includes better and more tests, bumping up to 100% line coverage.
Breaking changes
Bug Fixes
ferp.effects.thunk(method)
.subscriptions.every
,effects.delay
andeffects.raf
.Enhancements
ferp.effects.thunk(method)
.ferp.util.combineReducers
to manage deep nested reducers that return effects.Notes
Removing middleware
The current implementation of
middleware
allowed implementations to completely change the behaviour of ferp and the flow of effects, messages, and state. I attempted to build a functional alternative that I calledlisteners
, but felt it didn't really feel like a functional approach. After a few days of thinking, I opted to completely remove this mechanism - it was not a fit for the library, and just added an extra layer of complexity/debugging.Most middleware that I have thought of ends up feeling like they should be an effect, and in fact, the only middleware that I could justify was simply a logger that would output the message, and updated state, but this is only really useful in debug/early development of an application.
For these reasons, the mechanism has been removed. You can see examples of a high-order function logger in examples/cli if you find it necessary for debugging.
Removing results
I really liked the idea of result, being a hybrid of Elm's
Result
andMaybe
types. The problem is that it's a type; since I don't really have control over javascript, the result type isn't enforceable. Having something that reflects how Elm and other languages deal with nulls or async data states is neat, but I don't think it should be a core feature, especially because it is opt in. No internal code uses the result type, and my singular example of it was contrived.The code is hard to justify, too, considering anyone could just
npm install lodash.get
and have something akin toMaybe
support by using the default param.The code currently still exists in the latest push, but I have intentionally removed it from being exported until I can make a better decision on what I want to do with it.(update Sept 30, 2018: I removed it for now). Input welcome.