search

lukejacksonn / hyperapp-firebase-auth

:fire: Drop in authentication for hyperapps using firebase
https://codepen.io/lukejacksonn/pen/xLBJoN
MIT License
19 stars 4 forks source link

Upgrade to hyperapp 1.0 #2

Closed warwickgrigg closed 6 years ago

warwickgrigg commented 6 years ago

I've amended the code for the new concept of sliced state/actions. As "mixin" has gone in 1,0, I've created function wrappers which userland invokes to augment userland's state, actions and view before calling hyperapp's app with those. Please see index.sample.html for best demonstration.

lukejacksonn commented 6 years ago

Hey man! Thanks for taking the time to do this 🙇 I had already done this but forgot to push it up to GitHub 🙄 I have merged that in the state it is.. so your diff is going to look different now. Also this version is not going to export as a mixin anymore (obviously) and so the README is going to need updating like you did. I will welcome a PR for those or anything else you think need improving!

warwickgrigg commented 6 years ago

I'd appreciate a few pointers for proceeding:

  1. how you envisage the new API, the hyperapp.app invocation - with the s, a, view wrappers I proposed - or do you have something else in mind or would prefer?
  2. the status of the current code (PR3/PR4)
  3. the refs to "model" (lines 69, 71)
  4. an outline of what to "merge" from this PR2, what to keep from PR3/4, or vice versa, and what to create anew
  5. maybe we merge and test and prototype a new version in jsfiddle or codepen then pull/push to github?
lukejacksonn commented 6 years ago

Ok. Sorry about all of that. Everything was a mess! Should be fixed now.

  1. Here is a demo of importing now https://codepen.io/lukejacksonn/pen/xLBJoN
  2. Since PR 5/6/7 and the commits after. Status: 0.3.0 is working with hyperapp ^1.0.0
  3. Fixed, good spot, thanks!

So we can start a fresh from here.. if you would like to try it out and let me know it is working for you. Then feedback on anything you think is amiss as an issue or new PR, that would be awesome.

Thanks for spurring me on to do this.. it has been on my list for a while!

:bow:

warwickgrigg commented 6 years ago

That's great, thank you! Maybe you close off this PR now: I'm not sure of the convention whether you close it or if there's a way I withdraw it? This has been a useful learning experience for me for hyperapp, firebase, a functional VDOM pattern for auth, and rollup. I have some ideas for "improving" this module's API for invocation, getters etc; more elegant than my previous proposal I think. I'll float these ideas to you in separate request if that's OK? If you like the ideas I'll prepare a separate pull request.

lukejacksonn commented 6 years ago

That would be great! Glad it has been fun and educational. Thanks for the help/encouragement 💯 I will close here but feel free to open up another PR or issues if you find them :+1: