Open dalaing opened 6 years ago
I think this is a somewhat complex design decision - it seems easy to envision situations where auto-reloading is nice, but also pretty easy to envision situations where it would be annoying. In particular, I hit "save" an awful lot while editing code, and I'm sure sometimes I create valid states that aren't really a "final" state that I want to actually test. With auto-reload, that situation would cause my reference to go away, potentially losing the state of the page, e.g. what widgets I have open, etc.
Can we have it be configurable/discoverable easy? I'm also part of the saving frenzy club, but have in the past used steeloverseer and browsersync to get auto-compile + auto-reload before knowing or getting-to-work ghcid/jsaddle-warp
Actually making it reload should be reasonably easy with jsaddle - the trickier thing is just figuring out what the UX should be. If someone wants to take a stab at thinking through all the scenarios and figuring out what the reload logic ought to be, we can probably implement it pretty quickly.
👍 I think a live-reload option would be fantastic. Especially when you are making quick changes to the Frontend side of things (inline styles, etc)
the trickier thing is just figuring out what the UX should be
I would think that an option to ob run
would suffice: ob run --live-reload
would work well.
It seems like jsaddle might already support this? https://github.com/ghcjs/ghcjs-dom-hello/blob/master/src/HelloMain.hs#L28
[EDIT] Oops saw the OP had linked to some of this already
This might help :)
It doesn't effect the need to work out what the UX should be, but ob run --live-reload
looks pretty good to me :)
I'm taking a stab at this. The current structure of obeliskApp
doesn't lend itself well to using debugOr
since debugOr
returns IO ()
(because of the type of debugWrapper
). Unfortunately my first attempt failed with :
user error (Network.Socket.accept: can't accept socket ((AF_INET,Stream,6)) with status Closed)
which seemed pretty mysterious, almost like it was trying to use the port before it was open.
I copied in most of the code for debugOr
(as a test before upgrading deps) as another test but it broke the static routing for the image. However it does live reload 🎉. I think Im close, going to backtrack to see if I can get the static routing and other slightly non-standard jsaddle routing back in without too much changes to the logic that builds Application
I played around with this for a while, but ultimately I couldn't get past that socket accept error. It looks like its being thrown by the fallBackProxy
which explains why when I was running without the runSettingsSocket
and bindPortTCPRetry
in https://github.com/mjrussell/obelisk/commit/432389517ab3a1bd49e93c2903397b37d7d68d8e it was working fine (however no proxying to the backend). Here's the "more cleaned up" version that using runSettingsSock
but crashes when liveReload
is True
.
It looks to me like its an issue with the debugWrapper
from jsaddle playing with the rest of the code but Im at a bit of a loss. Maybe @ryantrinkle or others have some thoughts to nudge me in the right direction? I should be able to easily hook the plumbing in once I've gotten over this hurdle. (The latest is at https://github.com/mjrussell/obelisk/tree/run-live-reload, and I can also make a WIP PR if thats easier)
The ideal UX is hot module replacement, it's a reload with the state of the app remaining the same. In case you people are not aware.
With some success frameworks like cyclejs will record events- being frp-, and just rerun them after reload. I don't know what other's do, but a fine example is vuejs.
I have an experimental setup of obelisk used with live-reload thanks to browsersync. https://github.com/aveltras/obelisk-browsersync/blob/master/gulpfile.js
Two things need to be modified for this to work :
First, static assets path need to get their hash removed so browsersync can watch the static directory and correctly detect that an asset used on the page has changed. A possible fix would be to stop using hashes for the assets when in development mode (ob run could be considered development mode ?)
Second, the address of the websocket handler jsaddle talks to needs to point to the browsersync port and you need to configure browsersync to proxy websockets calls. To fix this, that call to run https://github.com/reflex-frp/reflex-dom/blob/develop/reflex-dom/src/Reflex/Dom/Internal.hs#L40 should be changed to lower level jsaddle warp functions such as that one https://github.com/ghcjs/jsaddle/blob/master/jsaddle-warp/src/Language/Javascript/JSaddle/WebSockets.hs#L177 which you can provide a url to.
Are those desirable fixes ? If so, i might take some time to make some pull requests.
Would it be possible to fold in something like this into
ob run
?I noticed the
False
at the end of this line (which would beTrue
with the debug machinery) and started wondering. I'm not quite clear on how best to squeeze in the rest of the debug stuff, but I can have a go in a PR if that's something you'd be keen on.