Adds buildOnSave command to allow full rebuild on save

[mikesol]

This is useful when doing live coding (using VSCode as a repl) and an entire project needs to be coherent in order to successfully reload in the browser. For example, during wagsi jam sessions, we need to make a call when to inject the current live code into the browser to update the music. If a single file changes (ie a function signature changes) and other files that use that function aren't updated, the hot reload will break the live session because the function will no longer be usable. Compiling everything all the time forces the entire graph to be coherent, which ensures a smooth hot reload.

[nwolverson]

Is it really that simple :)

May think on the setting name, I'm not sure it will be clear that it refers to doing a full build on save - it might be read as enabling building (at all) on save, and the fastRebuild as controlling how all building is done. fullBuildOnSave maybe

Actually setting both this and fastRebuild is a bit weird, they are somehow conflicting.

[wclr]

I think this needs a bit more of consideration.

First, I've thought about this feature being a bit more sophisticated. I checked this scenario and a full rebuild may be quite slow, even if almost no changes (say one file) it still takes a few seconds to start and finish the build. So a user won't get fast diagnostics feedback if it is desirable. So to ensure fast feedback LS first could do a fast rebuild first and then start performing the full rebuild, this would work better in terms of DX.

At the same time using fastRebuld and after that full build introduces a bit of inconsistency for hot-reload scenarios, but I think full-rebuild itself does not emit built artifacts in one the moment (if I guess it right how it works), so it is possible inconsistency for a short span there will be still, so using fast-rebuild + full-rebuild seems reasonable (though it could be made optional).

The second is more severe, the current change doesn't consider that multiple files can be saved at the same time in IDE, and this will trigger multiple build commands to be launched, which is a direct road to problems. The same problem with consequent savings of the same file while the build is still running. So there need to be introduced some mechanics to ensure that only one full build is running at the moment.

fullBuildOnSave maybe

I agree it is better to be more explicit here.

[wclr]

Just found it out, it turned out that a fast rebuild may work without touching the output. So the problem with the additional short-term inconsistency of fast rebuild results + full rebuild scenario can be made not actual.

It may be event won't need in certain scenarios: it should not be difficult to also implement a fast rebuild on typing (to get diagnostics while typing) without touching the output (and thus without triggering watchers and hot reloads).

This is due to fact that the fast rebuild purs-ide feature was designed quite flexibly, it doesn't require a target compiled module to be saved on disk directly (it may take an alternative file location to read the content) and also may not perform a codegen if asked not to (via codegenTargets), so the output won't be touched.

But the problem I found and not sure if is a bug or a feature: if to pass empty list ([]) as codegenTargets to the rebuild command, if the build is successful purs-ide won't touch/update the output JS files, but it still updates corresponding externs.cbor and cache-db.json hash as well. This seems strange as the code output is not updated, but the metadata is.

It also refers to the problem that even if the hash of the module (in cache-db) didn't change purs-ide still updates/touches corresponding externs that leads to unnecessary downstream rebuilds later.

@kritzcreek could you comment on the last points?

[mikesol]

The second is more severe, the current change doesn't consider that multiple files can be saved at the same time in IDE, and this will trigger multiple build commands to be launched, which is a direct road to problems. The same problem with consequent savings of the same file while the build is still running. So there need to be introduced some mechanics to ensure that only one full build is running at the moment.

I've seen this problem in my use case. I'm using this feature in hand-rolled purescript-language-server for several people doing live coding on the same repo, so I've seen before that 2-3 people can save stuff and then it will create a backlog of compilation. We'd want throttling here at a certain point. I think that, for a version 0.0.0 of the feature, my naive implementation would be fine (in that it's been livable so far in anger), but I'd be interesting in adding throttling to this and potentially other commands in the future.

[wclr]

I'm going to push my implementation of some other features regarding showing up diagnostics on typing, and it will contain this feature including throttling/debouncing and ensuring a single build running. So I propose to defer this one.

[nwolverson]

@wclr I'm concerned that it sounds like you're implementing the diagnostic on typing feature, which @i-am-the-slime has already demo'd, in a conversation you were involved with. Can you guys make sure you talk before going any further, whether on discord or by making an issue proposing the behaviour you have in mind.

[nwolverson]

The second is more severe, the current change doesn't consider that multiple files can be saved at the same time in IDE, and this will trigger multiple build commands to be launched, which is a direct road to problems. The same problem with consequent savings of the same file while the build is still running. So there need to be introduced some mechanics to ensure that only one full build is running at the moment.

The interesting part of this, if you hit "save all" with a bunch of file changes, you want to build after the last one if you're building everything, but you also don't want to be too unresponsive.

[wclr]

@wclr I'm concerned that it sounds like you're implementing the diagnostic on typing feature, which @i-am-the-slime has already demo'd, in a conversation you were involved with.

I wanted to try this feature long ago, I've finally got to this and implemented it to see if it can work without touching the output files. I believe this is the same way as was implemented by @i-am-the-slime, there are not many choices on how it should be implemented properly.

The interesting part of this, if you hit "save all" with a bunch of file changes, you want to build after the last one if you're building everything, but you also don't want to be too unresponsive.

If you have multiple files saved, the events will be firing very close, I think just debouncing the first event (with some default interval ~ 100 ms) and throttling others that occur during that interval should work fine.

[mikesol]

@nwolverson once you've merged those new features let me know and I can rebase the PR against them & test it out again.

[nwolverson]

I'm going to go ahead and merge this, I don't think it conflicts conceptually with diagnostics on typing (one can decide about each of those separately), I don't think it prevents enhancements such as debouncing the saves in case of save-all type behaviour, forcing the on-typing diagnostics before a build to see quicker errors in your current file, blocking multiple builds being run at once (this could already be an issue today).

I may mark it as experimental for now, as the configuration or the interaction with the other build features may be subject to change

[wclr]

I have no problem with this, it definitely prevents nothing (even running multiple builds on multiple files saved -).

[mikesol]

Sweet, thanks! If you could check out the companion PR at https://github.com/nwolverson/vscode-ide-purescript/pull/188, that'd allow this to get picked up in the vs code extension. Thanks!