Make the app's state immutable #426
Conversation
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 2 2
Lines 133 139 +6
Branches 40 39 -1
=====================================
+ Hits 133 139 +6
Continue to review full report at Codecov.
|
|
Other way to write this PR (by passing a path down the init()/initActions() functions + using getPath()/setPath() from #425) at this branch: https://github.com/Mytrill/hyperapp/tree/immutable-state-2 Gzipped size for this branch: 1535 bytes. |
|
It is going to take me a while to comprehend this.. do you have a branch/commit with comments included at all? 😅 (no worries if not) |
|
Comments please or just add here/below an explanation of what's happening in there! 🙏 /cc @Mytrill |
src/app.js
Outdated
| } | ||
|
|
||
| function globalUpdate(result) { | ||
| console.log("Calling global update with result: " + JSON.stringify(result)) |
There was a problem hiding this comment.
I guess this is here as a clue! 🕵️😅
|
I haven't checked the diff.. but I think I prefer the approach here https://github.com/Mytrill/hyperapp/tree/immutable-state-2 |
|
https://github.com/Mytrill/hyperapp/tree/immutable-state-2 is much easier to understand (pass the current path to init() and initActions() and always get(globalState, path) to get the current slice or set(globalState, path, result) to merge in the actions results. I will play around a bit more with both versions but probably end up doing another PR with the other branch, this one is too complicated, it's not worth the ~50 bytes difference (unless someone disagree, in which case I'll try to write down how the code in this PR works.). |
|
@Mytrill Can you just push on top of this PR? Don't push force, just add another commit with the changes. 🙏 |
src/app.js
Outdated
| globalState, | ||
| globalActions, | ||
| updateGlobalState, | ||
| getGlobalState |
There was a problem hiding this comment.
saves 3 bytes if we inline getGlobalState with
function() {
return globalActions
}
Minified uncompressed
Before 3349 bytes
After 3346 bytes
There was a problem hiding this comment.
Done in the next commit.
| // do not override state is already exist in current module | ||
| state[key] || (state[key] = {}), | ||
| // do not override actions is already exist in current module | ||
| actions[key] || (actions[key] = {}), |
There was a problem hiding this comment.
@Mytrill My PR doesn't have this, is it necessary?
|
Thanks @Mytrill! 🎉 |
This PR addresses #418 and #425.
The approach here is to recursively modify the
updatefunction as we initialize the actions/modules.EDIT1: Gzipped size (without the console.log...): 1476 bytes
EDIT2: after commit 7b839a5 size is: 1492 bytes (but the code is easier to understand IMHO)
EDIT3: 1487 bytes (with fix to #438)