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

Add hot reload for the watch command (#627) #629

Merged
merged 6 commits into from
Jan 13, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions src/FSharp.Formatting.CommandTool/BuildCommand.fs
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,24 @@ type internal DocContent(outputDirectory, previous: Map<_,_>, lineNumbers, fsiEv

/// Processes and runs Suave server to host them on localhost
module Serve =
//not sure what this was needed for
//let refreshEvent = new Event<_>()

let refreshEvent = new Event<_>()
let mutable signalHotReload = false

let socketHandler (webSocket : WebSocket) _ = socket {
while true do
do!
refreshEvent.Publish
|> Control.Async.AwaitEvent
|> Suave.Sockets.SocketOp.ofAsync
do! webSocket.send Text (ByteSegment (Encoding.UTF8.GetBytes "refreshed")) true
let emptyResponse = [||] |> ByteSegment
//not sure what this was needed for
//do!
// refreshEvent.Publish
// |> Control.Async.AwaitEvent
// |> Suave.Sockets.SocketOp.ofAsync
//do! webSocket.send Text (ByteSegment (Encoding.UTF8.GetBytes "refreshed")) true
if signalHotReload then
printfn "Triggering hot reload on the client"
do! webSocket.send Close emptyResponse true
signalHotReload <- false
}

let startWebServer outputDirectory localPort =
Expand Down Expand Up @@ -643,33 +651,43 @@ type CoreBuildOptions(watch) =
if not docsQueued then
docsQueued <- true
printfn "Detected change in '%s', scheduling rebuild of docs..." this.input
Async.Start(async {
async {
do! Async.Sleep(300)
lock monitor (fun () ->
docsQueued <- false
if runDocContentPhase1() then
if runDocContentPhase2() then
regenerateSearchIndex()
) }) )
)
}
|> Async.RunSynchronously
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has changed from Start to RunSynchronously. If it's RunSynchronously then no async is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, this is unnecessary. I was kind of thrown off by the 'fire and forget' nature of Async.Start (via this) and ditched it without second thought thinking i have to wait until the computation is finished. I missed that the hot reload trigger does not have to happen after the whole block but just after the last function call inside it, so i moved it inside the block now using the original Async.start version again.

Serve.signalHotReload <- true
)

let apiDocsDependenciesChanged = Event<_>()
apiDocsDependenciesChanged.Publish.Add(fun () ->
if not generateQueued then
generateQueued <- true
printfn "Detected change in built outputs, scheduling rebuild of API docs..."
Async.Start(async {
async {
do! Async.Sleep(300)
lock monitor (fun () ->
generateQueued <- false
if runGeneratePhase1() then
if runGeneratePhase2() then
regenerateSearchIndex()) }))
regenerateSearchIndex())
}
|> Async.RunSynchronously
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise here....

Serve.signalHotReload <- true
)

// handler for signalling to refresh the page to the client
let hotReloadHandler _ = Serve.signalHotReload <- true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this hotReloadHandler get used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, that is indeed a leftover from a previous test, will remove


// Listen to changes in any input under docs
docsWatcher.IncludeSubdirectories <- true
docsWatcher.NotifyFilter <- NotifyFilters.LastWrite
docsWatcher.Changed.Add (fun _ ->docsDependenciesChanged.Trigger())
docsWatcher.Changed.Add (fun _ -> docsDependenciesChanged.Trigger())

// When _template.* change rebuild everything
templateWatcher.IncludeSubdirectories <- true
Expand Down