vscode-reasonml icon indicating copy to clipboard operation
vscode-reasonml copied to clipboard

bsb integration can break separate bsb watch process with different versions

Open ChristianHersevoort opened this issue 7 years ago • 3 comments

The vscode-reasonml bsb integration isn't isolated from other separate bsb processes. The editor can even corrupt a separate bsb watch process with a different bsb version. The editor bsb process shouldn't interfere with anything and it shouldn't perform it's tasks in the same (output) directory as the project.

Some good news: corruption due to different versions of bsb shouldn't happen anymore because ; PR #210 changes the default to always use the local version of bsb. But the actual underlying issue isn't fixed. Manual workaround: Add the following setting in VSCode;
"reason.path.bsb": "./node_modules/bs-platform/lib/bsb.exe",

Steps to reproduce;

  1. Install VSCode reasonml plugin (obviously)
  2. Install global bsb (for example 3.0.0) and reason-cli (any version should work).
  3. Configure VSCode settings "reason.diagnostics.tools": [ "merlin", "bsb"] (see readme)
  4. Initialize a new ReasonReact project with bs-platform 3.1.4. It's important that it's not the same version as the global bsb version. (You should now have 2 version installed; 3.0.0 global and 3.1.4 local)
  5. Start bsb watch process; npm start (or ./node_modules/bs-platform/lib/bsb.exe -make-world -w)
  6. Edit any ReasonReact component, make many small changes and save it often (e.g. add a space, save, add another space, save, etc.) you might need to repeat it many times because this issue triggers randomly.
  7. The bsb watch process corrupts and throws and error with the message "The module or file ReasonReact can't be found." This error is "fatal" because bsb cannot recover; cleaning and restarting the bsb watch process is the only option.

Expected result: The editor bsb process shouldn't preform it's tasks in the same (output) directory. The editor integration shouldn't interfere with anything inside the project. Maybe it should use a temp directory?

Questions Shouldn't the bsb lock file prevent these kinds of issues?? This issue might also explain some other weird editor bugs, for example errors showing only every other save ?

ChristianHersevoort avatar May 26 '18 18:05 ChristianHersevoort

Thanks for the report. Unfortunately I don't actually use Bucklescript regularly right now and don't have much to say about this issue on a technical level. Maybe @jchavarri or @chenglou would comment?

However, my take on the issue of bsb integration with regard to coordinating multiple processes is that this should really be the responsibility of the Bucklescript tooling itself rather than the language server. As a separate project, I don't think it's feasible for the language server to keep in sync with the behavior of the Bucklescript tooling at this level of granularity, at least not unless the Bucklescript devs want to take ownership of the integration with the language server.

Having said that, we should do what we can to avoid issues, and if there are easy fixes that someone would like to submit a patch for I would be happy to help and get them merged.

Another thing to mention here is that I think at one point @bobzhang had suggested creating a server interface for bsb itself. This would be something we could communicate with via JSON or whatever, similar to how we currently interface with merlin. This would be a better solution than us having to launch processes, etc.

The editor bsb process shouldn't preform it's tasks in the same (output) directory. The editor integration shouldn't interfere with anything inside the project. Maybe it should use a temp directory?

I don't think this is feasible in general given how some of the editors work. VS code in particular will expect to be able to read/write some data to the project directly in certain cases. We don't make use of that now but we might at some point. Running processes from temporary directories may be an option though.

Shouldn't the bsb lock file prevent these kinds of issues?? This issue might also explain some other weird editor bugs, for example errors showing only every other save ?

I really don't know. This is a question for @bobzhang. Maybe a separate issue should be created in the Bucklescript repo.

ghost avatar May 27 '18 00:05 ghost

cc @jaredly

chenglou avatar May 27 '18 06:05 chenglou

  1. Start bsb watch process; npm start (or ./node_modules/bs-platform/lib/bsb.exe -make-world -w)

This is the main DX issue imo: users tend to keep running bsb watcher manually after adding bsb to reason.diagnostics.tools, but it's not necessary: the watcher does exactly the same thing as the editor integrated bsb process, once a file is saved, it'll re-compile. Maybe we could detect from the lang server if there's an instance of the watcher running and inform / explain users via an in-editor notification? That would solve a large % of cases where there is confusion because one instance of bsb is stepping on the other.

jchavarri avatar May 27 '18 21:05 jchavarri