Pnpm and other hard cache based managers cannot work with patch

[BusinessDuck]

Hi! As you may remember we had a discuss about pnpm and patching here.

I know the reason of this error:

. postinstall$ monaco-treemending
. postinstall: Error: Unable to apply patch on 

Inside of pnpm we have hard package cache and three-awesome collecting all packages in to the one flat store without dependencies (symlinks used inside of internal packages deps folder) for example:

monaco-language-client depends from monaco-editor

dependencies": {
    "monaco-editor": "~0.37.1",
...  },
  "peerDependencies": {
    "vscode": "npm:@codingame/monaco-vscode-api@~1.78.6",
    ...

pnpm will extract this packages and create single for monaco-language-client and root package.json of main project. That mean if monaco-langauge-client contains postinstall script, the patch will be applied to symlinked package. Then second call postinstall in project root inside package.json gonna be fall.

Possible solutions:

Remove postinstall from monaco-languageclient and move out from package main packgage.json to examples or something other dir

Also we need to fix treemending script patch already installed detection and show the info, "Already patched", in case when patch really installed

Patch listing

diff --git a/package.json b/package.json
index 88c2d0ac8cac849a09c7bb534e52669323e1d7f1..dc881bc2bc5034b532429947780accd498bb7002 100644
--- a/package.json
+++ b/package.json
@@ -66,7 +66,6 @@
     }
   },
   "scripts": {
-    "postinstall": "monaco-treemending",
     "clean": "npx shx rm -fr lib *.tsbuildinfo",
     "compile": "tsc --build tsconfig.src.json",
     "build:msg": "echo Building monaco-languageclient:",

UPD: Patch didnt works for me :C

[BusinessDuck]

pnpm i --ignore-scripts works fine. Package monaco-editor didn't patched by monaco-language-client.

Can you please move out or remote postinstall from monaco-language-client, is that possible?

[kaisalmen]

@BusinessDuck a fix for detecting treemending was already applied is in the works (CodinGame/monaco-vscode-api#115), but we are discussing the best wayforward currently. Then calling it n times will no longer cause problems.

[BusinessDuck]

I create my own patch by using pnpm patch cli, that works pretty. pnpm patch ist like patch-package, but out of the box. I dont know why u guys didn't use that patch software, but i see you calculates hashes manually, why? Patch-packgage already works with all corner cases inside of patch applying mechanism

[kaisalmen]

patch-package, but out of the box. I dont know why u guys didn't use that patch software

Thanks for the hint. @CGNonofr what do you think about using https://www.npmjs.com/package/patch-package

[BusinessDuck]

Example with pnpm (i guess contracts are same)

Step 1: Create patch
pnpm patch monaco-editor@0.37.1

That will produce tmp folder to work with it for doing some modifications with package code. When you done with it go to step 2

Step 2: Commit patch
pnpm patch-commit {folder path that you receive on step 1}

That will produce new .patch file and modify package.json

[CGNonofr]

I've considered it but AFAIK, patch-package only allows to patch a package from the final project, generating a patch file in a specific folder. Please tell me how to use it in our use case.

[BusinessDuck]

pnpm make changes inside of package.json, when i use pnpm i, patches will be executed automatically.

in case with patch-package i guess https://github.com/ds300/patch-package#applying-patches

[CGNonofr]

Ok so with pnpm I guess you can use it, referencing monaco-editor-treemending.patch from monaco-vscode-api

"patchedDependencies": {
  "monaco-editor@0.37.1": "node_modules/vscode/monaco-editor-treemending.patch"
}

?

[BusinessDuck]

I cant use patch linked to package, because patch cannot be found before node_modules/vscode/ installed.

[CGNonofr]

So what are you suggesting here?

[BusinessDuck]

Move patch to the client side or completely fork monaco-editor to monaco-editor-vscode (patched) package

[BusinessDuck]

Patch is absolutely same with fork in current case, because patch contains a lot of changes... so much... :)

And patch still not be able to apply any other monaco-editor version. That mean freeze (same with fork) monaco-editor package version.

Also patch solution cannot be validated like forked typescript project. You can have bugs inside of patched code and you will not see this like in typescript

[CGNonofr]

Move patch to the client side

What do you mean?

completely fork monaco-editor to monaco-editor-vscode (patched) package

I've considered it, but it wouldn't allow to use other libraries using monaco-editor, like monaco-vim or monaco-emacs

[BusinessDuck]

I've considered it, but it wouldn't allow to use other libraries using monaco-editor, like monaco-vim or monaco-emacs

Use react and preact way with aliases. If shared public contracts and exports is absolutely same between you fork and monaco-editor, everyone other who use monac-emacs can just use package aliases. See preact readme guide https://preactjs.com/guide/v10/getting-started#aliasing-react-to-preact

[CGNonofr]

We had a fork of monaco-editor before, and we decided to give it up, it has some cons:

• We need to release a version of the fork for every version of the official monaco-editor (it can't be automated, and we would need to release both monaco-editor-core and monaco-editor)
• npm aliasing doesn't work very well when other libraries have monaco-editor as dependency and it creates conflicts
• webpack/rollup... aliases require configuration in the final project

At the end, it's simpler for the final user to include the postinstall script than requiring them to use a fork of monaco

[BusinessDuck]

Okay lets try do preinstall patch downloading from remote package. That may works fine with.

....
"scripts": 
    "preinstall": "monaco-patch-loader",
 },
 ....
"patchedDependencies": {
  "monaco-editor@0.37.1": "node_modules/.npm-patches/monaco-editor-treemending.patch"

monaco-patch-loader - should keep patch up to date and pre-serve patch file for post-install phase. Than remove tmp dir after patch installation on post-install

[kaisalmen]

@BusinessDuck forking monaco-editor because we have a tool specific issue is not a good idea, IMO. In the end we create even more problems.

If I recall correctly from what you wrote above postinstall did not fail the first time. I favor an approach (hash based or not) that doesn't rely on other packages or a specific package manager. It must solely rely on node.js and work cross-plattform. The postinstall script must just not complain if everything is fine and that was wrong from the start. Sorry for this.

[BusinessDuck]

Please no apologies, this is an open source project.

If none of the options I suggested are suitable, I think that this is normal, because I am not familiar with many of the nuances in your project.

Thanks, I'll be waiting for the merge. Perhaps I will contribute to this project later.

[BusinessDuck]

Do you have chat with Microsoft guys? Can i join?

[kaisalmen]

Do you have chat with Microsoft guys? Can i join?

There are no plans currently.