ts-patch
ts-patch copied to clipboard
[Feature] Add mutex to `ts-patch install`
I'm using ts-patch
in a monorepo with rushjs
and pnpm
. When installing multiple projects (=multiple package.json
files) at once, the prepare
script calling ts-patch install -s
in each of the package.json
files can cause havoc. PNPM does magic with symlinks, so many projects will share the same dependency files.
In summary, the PNPM installation goes something like this:
- Download and install all dependencies across all projects in the monorepo
- Run
prepare
script on all projects in the monorepo at the same time (=parallel execution)
This leads to incidents where the patch to node_modules/typescript
is applied incorrectly or partially. ts-patch
has a check for already patched files, but if two prepare
scripts run simultaneously, that check can be missed.
In order to fix this reliably, I applied a mutex to all the prepare
scripts in my monorepo, changing the prepare
from this:
{
"scripts": {
"prepare": "ts-patch install -s"
}
}
To this:
{
"scripts": {
"prepare": "acquire-mutex && ts-patch install -s && release-mutex"
}
}
That has fixed the patch issues for me. Ideally the mutex should be part of ts-patch
though, so everyone using PNPM/Rush.js wouldn't have to write their own mutex scripts.
Error example:
/path/to/common/temp/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/tsc.js:117780
ts.diagnosticMap.set(program, allDiagnostics);
TypeError: Cannot read properties of undefined (reading 'set')
Great point. Thanks for raising this.
The main option with node would be to use file locking, but it turns out this is a little tricky in terms of multi-OS support. fs-ext
added support as of Node ~ 8.9, but there were various recent bug reports of it not working on later node, and it also requires python + node-gyp
just to install, as it goes through a build process. If you're missing one of these components, it throws a nasty error when you try to install. Unfortunately, this makes it not a viable choice.
What I ended up doing was simply creating a temporary file in the destination directory (the specific ts package path). With this route, we essentially indicate that this ts install path is "locked". I also added logic so that it will retry up to 3 times with 10sec pause. This should alleviate any issues.
While we do have a proper finally
block which ensures it gets "unlocked", I've also added a --force
flag which will skip the mutex check in case the file doesn't get deleted in some cases. The final throw after 3 attempts indicates this, as well.
That said, I think the bases are pretty well covered in this approach. Feel free to weigh in if you have any thoughts.
All that is left to do is add in some proper tests, and I'll get a release together.
Is there any progress in this matter ?
Added in v3