ionic-team / vscode-ionic

Visual Studio Code Ionic Extension
Other
5 stars 0 forks source link

"packageManager" entry is added to "package.json" by the extension #151

Closed triplestepab closed 1 month ago

triplestepab commented 1 month ago

When the plugin is enabled in vscode and opening a project containing package.json, the following line is added:

"packageManager": "yarn@1.22.22+sha1.ac34549e6aa8e7ead463a7407e1c7390f61a6610"

It might have to do with the extension referencing corepack, because in the logs I can see

Corepack will now add one referencing yarn@1.22.22+sha1.ac34549e6aa8e7ead463a7407e1c7390f61a6610

Disabling the extension Ionic.ionic removes this behaviour.

Deleting the line and reopening the project immediately adds the line back.

dtarnawsky commented 1 month ago

Yarn expects a packagemanager field in package.json to define which version of yarn it should use.

Is this behavior causing a problem? as it is pretty typical to have this for a yarn based project.

triplestepab commented 1 month ago

The project doesn't use Yarn and even if it did, Ionic.ionic (itself or a subcomponent it uses) should not modify this setting.

Think like this; it would be pretty intrusive if a user installed a vscode plugin and it modified project files (unless, of course, it's the purpose of the plugin).

patrick-entinux commented 1 month ago

I'm not using yarn v1 and I have gotten the same experience.

One weird issue recently was I did:

pnpm patch @turf/buffer

Note: pnpm patch is a command to prepare small patches of npm packages.

I opened the patch workspace in a new VSCode window. So this workspace only contains @turf/buffer and nothing else. When I finished preparing the patch, there was a surprise line packageManager in the final patch of @turf/buffer/package.json:

diff --git a/package.json b/package.json
index f350d3e54b0a2e5c0bdc82955cb69af22f150657..5903c40dba3caf0a1db8904c8ba7466b1831462a 100644
--- a/package.json
+++ b/package.json
@@ -35,6 +35,7 @@
   "exports": {
     "./package.json": "./package.json",
     ".": {
+      "types": "./index.d.ts",
       "import": "./dist/es/index.js",
       "require": "./dist/js/index.js"
     }
@@ -71,5 +72,6 @@
     "d3-geo": "1.7.1",
     "turf-jsts": "*"
   },
-  "gitHead": "5375941072b90d489389db22b43bfe809d5e451e"
+  "gitHead": "5375941072b90d489389db22b43bfe809d5e451e",
+  "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
 }

Luckily it did not cause any lasting issues but it's not ideal to have unrelated lines in this kind of patch.

dtarnawsky commented 1 month ago

Do you have a sample repo that exhibits this behavior? The extension doesn't add a packageManager field to package.json, but it will use it if it is there to know which version of yarn to use.

So I'd be curious about what command is causing this to be added. My guess is that this is actually node 20 doing this because of changes in corepack.

patrick-entinux commented 1 month ago

@dtarnawsky I don't have a sample repo that would show it well but it can be observed with just a package.json file, so the following script works to reproduce it:

mkdir my-project
cd my-project
# Create a package.json file
npm init -y
# Print the package.json file, note there is no packageManager line yet
cat package.json
# Open VS Code
code .

If I have the Ionic VSCode extension enabled, after opening VS Code, I see the following line in the new package.json:

"packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"

If the extension is disabled, it does not get that line at all.

I have corepack enabled and my Node version is v20.12.2

Usually if it asks "do you want to install node modules"? I say no. I also tried answering "never" and tested again, and it still added packageManager. I tried making my package.json readonly via chmod -w package.json to see if I could get the extension to throw an error when there is a write to the package.json file but it didn't work, unfortunately.

dtarnawsky commented 1 month ago

The packageManager flag is added when running yarn --version.

Can you try updating your version of yarn? also try that command at the terminal to see if that is the exact yarn command causing this.

patrick-entinux commented 1 month ago

My answer to your question follows but before that, is the Ionic plugin running yarn --version? While researching to answer your question I found that yarn --version indeed sets packageManager field, but by setting the env variable COREPACK_ENABLE_AUTO_PIN=0 this behavior will be suppressed, eg:

COREPACK_ENABLE_AUTO_PIN=0 yarn --version

Maybe it is possible to do the same inside the Ionic plugin?

It is documented here: https://github.com/nodejs/corepack/blob/main/README.md#environment-variables


I don't have yarn "officially" installed as I am using corepack:

➜  test-3 npm ls -g
/Users/pm/Library/Application Support/fnm/node-versions/v20.12.2/installation/lib
├── corepack@0.28.1
├── npm@10.8.0
└── nx@19.0.7

Originally the corepack global yarn version was 1.22.22:

➜  dev yarn --version
1.22.22

Indeed when I try running yarn --version in a fresh project directory it told me it was updating the packageManager field

➜  test-2 yarn --version
! The local project doesn't define a 'packageManager' field. Corepack will now add one referencing yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e.
! For more details about this field, consult the documentation at https://nodejs.org/api/packages.html#packagemanager

1.22.22

I had corepack update it's global yarn version to latest:

➜  dev corepack install --global yarn@latest
Installing yarn@4.2.2...

Even so, no change in behavior, still updated the package json:

➜  test-3 yarn --version
! The local project doesn't define a 'packageManager' field. Corepack will now add one referencing yarn@4.2.2+sha512.c44e283c54e02de9d1da8687025b030078c1b9648d2895a65aab8e64225bfb7becba87e1809fc0b4b6778bbd47a1e2ab6ac647de4c5e383a53a7c17db6c3ff4b.
! For more details about this field, consult the documentation at https://nodejs.org/api/packages.html#packagemanager

4.2.2
dtarnawsky commented 1 month ago

Hey Patrick, as mentioned in the corepack docs "In general we recommend to always list a packageManager field....as it ensures that your project installs are always deterministic"

When you have a packageManager field it does ensure that package manager is used, without it the extension has to make guesses as to what package manager you may want by looking for yarn, pnpm, bun, turborepo, npm, etc which takes a lot of time and is often incorrect. If you are going to enable corepack, it's your intention to use it and having a packageManager set is how it works.

We wouldn't have the extension disable the default behavior of corepack.

triplestepab commented 1 month ago

Do you have a sample repo that exhibits this behavior? The extension doesn't add a packageManager field to package.json, but it will use it if it is there to know which version of yarn to use.

I cannot reproduce anymore (I would say an update has resolved this), but to be super clear: The issue had nothing to do with node20 as it was 100% connected to the usage of the ionic plugin:

Also, yarn wasn't in my system (at least not in my path): "zsh: command not found: yarn"

I figure if nobody else can reproduce we close this issue.