Soft-disable WorkerManager…

[savetheclocktower]

…in order to skip the unnecessary creation of a new renderer process. Fixes #44.

This is the quickest way to neutralize #44. Since this package has an excellent fallback strategy when WorkerManager fails to produce a worker, the easiest way for us to “fix” this is to bypass WorkerManager more thoroughly. This means preventing the attempt to spawn a worker in the first place, since we know it will fail.

I've been using PulsarNext for more than a year, including tons of Git operations. My quixotic effort to rewrite the github package in SolidJS with modern idioms has been done entirely in PulsarNext. It's clear that the fallback strategy of just calling dugite’s GitProcess.exec directly is working just fine (at least for me), so I think we can proceed with it indefinitely.

A more proper fix here would involve removing all the WorkerManager code altogether. But the hoops we jump through in order to keep this package running motivated me to make the smallest possible change instead.

Testing

This is where it gets fun. Even I haven't tested this change because of the pain involved in pre-transpiling.

This change makes it so that we hit the same code path we were hitting before (direct usage of dugite) — but without implicitly trying to create a new BrowserWindow first. That's why we have a new check for WorkerManager.isEnabled() (which just checks a boolean) before the WorkerManager.getInstance() call later in the conditional clause (which will trigger the creation of a BrowserWindow — or at least did previously; now the Worker constructor also checks the ENABLED flag as a matter of belt-and-suspenders caution).

My preference is that, if this change seems logically sound, we can land it and then generate a new pretranspiled release branch — then I can test it pretty easily at that point to verify that we're skipping all doomed efforts to create BrowserWindows. If not, then we don't have to bump the github package version in Pulsar, and I can return to the drawing board.

[confused-Techie]

While largely this code seems sound (obviously not without any testing as you've already described), I do think there may be some value in adding something within the comment in front of the declaration of ENABLED to clearly say that we are disabling the WorkerManager entirely.

Since there's no place we ever allow ENABLED to become true it's completely disabled, which sounds like the point of this PR. But maybe a simple This constant intentionally disables the WorkerManager entirely per: pulsar-edit/github#45 just so we don't ever have to scratch our heads in the future or accidentally enable this in some other way without knowing the implications.

Otherwise, happy to say this seems completely valid

[savetheclocktower]

OK, updated the comment to explain that this is a permanent change and that we'll eventually remove WorkerManager.

[confused-Techie]

This looks perfect! Thanks for taking the feedback, I just hate having to dig in a diff to find out what an intention is.
Looks great to me