Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GENERAL: Only allow PNPM #366

Closed
kriptonian1 opened this issue Jul 25, 2024 · 22 comments · May be fixed by #467
Closed

GENERAL: Only allow PNPM #366

kriptonian1 opened this issue Jul 25, 2024 · 22 comments · May be fixed by #467
Assignees
Labels
difficulty: 1 foss hack Clustering all the curated issues for Foss Hack 2024 good first issue Good for newcomers help wanted Extra attention is needed priority: low scope: general Anything related to this repository type: chore Release drafter tag for tagging PRs related to making patches to general code

Comments

@kriptonian1
Copy link
Contributor

Description

We have noticed many of the contributors are using NPM instead of using PNPM. So we want to have a strict restriction over the package manager to be use for this project, and throw error if they are using any other package manager. To only allow PNPM, you can refer to this section of PNPM Doc to set it up for us.

@rajdip-b
Copy link
Member

We need to make sure that npx works after this.

@kriptonian1
Copy link
Contributor Author

kriptonian1 commented Jul 25, 2024

@rajdip-b we don't use npx in pnpm for pnpm it's pnpm dlx or pnpx docs

@rajdip-b
Copy link
Member

@rajdip-b we don't use npx in pnpm for pnpm it's pnpm dlx or pnpx docs

Yep i know but i think in some places i think we are using npx?

@kriptonian1
Copy link
Contributor Author

Then it should be replaced with pnpm dlx because it is not the right way of using it

@rajdip-b rajdip-b added help wanted Extra attention is needed type: chore Release drafter tag for tagging PRs related to making patches to general code foss hack Clustering all the curated issues for Foss Hack 2024 and removed enhancement labels Jul 25, 2024
@Z-xus
Copy link
Contributor

Z-xus commented Jul 26, 2024

/attempt fosshack

Copy link

Assigned the issue to @Z-xus!

@Z-xus
Copy link
Contributor

Z-xus commented Jul 26, 2024

It seems there's an ongoing issue which doesn't work for npm but only for yarn pnpm/only-allow#27

@kriptonian1
Copy link
Contributor Author

Let me have a look what it says

@MelloB1989
Copy link

/attempt

Copy link

Assigned the issue to @MelloB1989!

@Z-xus
Copy link
Contributor

Z-xus commented Jul 27, 2024

Assigned the issue to @MelloB1989!

@rajdip-b

@rajdip-b
Copy link
Member

Hey @MelloB1989, please check if the issues are already assigned or not. You are only allowed to take up issues in case they are not assigned to anyone for FOSS Hack.

@Z-xus
Copy link
Contributor

Z-xus commented Jul 27, 2024

@rajdip-b @kriptonian1
The underlying issue is with npm, which does not respect the preinstall script and executes it postinstall. refer: npm/cli#2660
I propose that a single shell script which automates the build process could be a probable solution to this since the root issue is untouched from years.

@rajdip-b
Copy link
Member

@kriptonian1 thoughts?

@Z-xus can you give a slight example of how this script will work?

@kriptonian1
Copy link
Contributor Author

Maybe we can do use .npmrc to make the engine strict. ref : https://www.freecodecamp.org/news/how-to-force-use-yarn-or-npm/

@Z-xus
Copy link
Contributor

Z-xus commented Jul 28, 2024

  1. Tested this but it didn't work as intended, perhaps it was version specific. https://www.freecodecamp.org/news/how-to-force-use-yarn-or-npm/

  2. I was proposing a shell script to automate the project setup which performs dependency checks, and then followed by step by step commands, although this is quite simple in this project and I feel it's a bit unnecessary since the setup is relatively simple

command_exists() {...}
check_dependencies() {...}
install_dependencies() {...}

@Z-xus
Copy link
Contributor

Z-xus commented Jul 28, 2024

By unintended I mean, npm and yarn weren't working but that was entirely due to different errors (my guess is that it is some specific package.json rule which is only applicable to pnpm):

image

@kriptonian1
Copy link
Contributor Author

ig that is corepack doing it for us, so I feature I don't like about corepack is that it is a bit too strict with the version of package manager

@kriptonian1
Copy link
Contributor Author

you can refer to this to learn how corepack work https://www.totaltypescript.com/how-to-use-corepack

I think we can just use it to restrict users to use pnpm, but the problem with this is it won't even allow to run command if the version of your pnpm dosen't match. so maybe you can look into this

@muntaxir4
Copy link
Contributor

Description

We have noticed many of the contributors are using NPM instead of using PNPM. So we want to have a strict restriction over the package manager to be use for this project, and throw error if they are using any other package manager. To only allow PNPM, you can refer to this section of PNPM Doc to set it up for us.

Although, other package managers will show error due to presence of workspace:*. If just-npm (pnpm/only-allow#27 (comment) ) is installed in project's root this will prevent any installation through npm in child packages as well.

@rajdip-b
Copy link
Member

rajdip-b commented Nov 10, 2024

I'll give this a try.

UPDATE: I tried it, seems like it doesn't work. I guess we should close this issue.

@muntaxir4
Copy link
Contributor

I'll give this a try.

UPDATE: I tried it, seems like it doesn't work. I guess we should close this issue.

I tested it on my end and it was working fine, except for the workspace:* issue which didn't let npm install run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: 1 foss hack Clustering all the curated issues for Foss Hack 2024 good first issue Good for newcomers help wanted Extra attention is needed priority: low scope: general Anything related to this repository type: chore Release drafter tag for tagging PRs related to making patches to general code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants