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

Not all package managers are respected #1219

Open
moltar opened this issue Dec 14, 2023 · 7 comments
Open

Not all package managers are respected #1219

moltar opened this issue Dec 14, 2023 · 7 comments
Labels
help wanted Accepting PRs package managers Related to package manager support

Comments

@moltar
Copy link
Contributor

moltar commented Dec 14, 2023

It looks like only yarn and npm are handled, but pnpm (and others) are not.

const addDependencies = async () => {
const yarnRoot = findYarnWorkspaceRoot(c.root) || c.root
if (existsSync(path.join(yarnRoot, 'yarn.lock'))) {
await copy(path.join(yarnRoot, 'yarn.lock'), path.join(c.workspace(), 'yarn.lock'))
const {stdout} = await exec('yarn -v')
const yarnVersion = stdout.charAt(0)
if (yarnVersion === '1') {
await exec('yarn --no-progress --production --non-interactive', {cwd: c.workspace()})
} else if (yarnVersion === '2') {
throw new Error('Yarn 2 is not supported yet. Try using Yarn 1, or Yarn 3')
} else {
try {
await exec('yarn workspaces focus --production', {cwd: c.workspace()})
} catch (error: unknown) {
if (error instanceof Error && error.message.includes('Command not found')) {
throw new Error('Missing workspace tools. Run `yarn plugin import workspace-tools`.')
}
throw error
}
}
} else {
const lockpath = existsSync(path.join(c.root, 'package-lock.json'))
? path.join(c.root, 'package-lock.json')
: path.join(c.root, 'npm-shrinkwrap.json')
await copy(lockpath, path.join(c.workspace(), path.basename(lockpath)))
await exec('npm install --production', {cwd: c.workspace()})
}
}

Related:

I think this should be solved with a proper interface that each package manager solution can implement.

@mdonnalley
Copy link
Contributor

@moltar I agree that this should support pnpm but I don't think we have the bandwidth at the present moment to implement a solution. However, we'll take a PR if you need a solution now and it's something you're willing to take on.

@moltar
Copy link
Contributor Author

moltar commented Dec 14, 2023

Would you accept the use of this dependency? https://www.npmjs.com/package/detect-package-manager

@mdonnalley
Copy link
Contributor

Would you accept the use of this dependency? https://www.npmjs.com/package/detect-package-manager

Yeah looks fine to me

@ericis
Copy link
Contributor

ericis commented Dec 28, 2023

Would you accept the use of this dependency? https://www.npmjs.com/package/detect-package-manager

This doesn't appear to respect the "package.json" packageManager setting.

Example:

"packageManager": "yarn@4.0.2",

For background, we attempted to change an oclif CLI implementation embedded as a subdirectory of a monorepo with pnpm at its root. For a while, the only option was to simply have pnpm ignore the oclif directory contents entirely and to use the classic yarn v1 in the oclif project. Recently, while testing an upgrade to yarn v4, we refactored the base monorepo to yarn as well and abandoned pnpm simply because switching was unnecessary friction and, while pnpm is awesome, there wasn't enough justification not to use yarn instead and make the whole thing work consistently. *We still had to configure yarn to use classic mode by setting the file ".yarnrc.yml" with the following:

# IMPORTANT: oclif expects classic dependencies
nodeLinker: node-modules
nmHoistingLimits: dependencies

@mdonnalley mdonnalley added help wanted Accepting PRs package managers Related to package manager support labels Mar 5, 2024
@kurtaking
Copy link

Would you accept the use of this dependency? https://www.npmjs.com/package/detect-package-manager

This doesn't appear to respect the "package.json" packageManager setting.

Example:

"packageManager": "yarn@4.0.2",

For background, we attempted to change an oclif CLI implementation embedded as a subdirectory of a monorepo with pnpm at its root. For a while, the only option was to simply have pnpm ignore the oclif directory contents entirely and to use the classic yarn v1 in the oclif project. Recently, while testing an upgrade to yarn v4, we refactored the base monorepo to yarn as well and abandoned pnpm simply because switching was unnecessary friction and, while pnpm is awesome, there wasn't enough justification not to use yarn instead and make the whole thing work consistently. *We still had to configure yarn to use classic mode by setting the file ".yarnrc.yml" with the following:

# IMPORTANT: oclif expects classic dependencies
nodeLinker: node-modules
nmHoistingLimits: dependencies

Hey @ericis, could you provide insight into how you handle yarn workspace dependencies when running oclif pack tarballs?

'dependencies': {
  'example-pkg': 'workspace:^'
}

And does your monorepo have a single .yarnrc.yml file at the root with nmHoistingLimits: dependencies or is that only defined in your version of the packages/cli?

@ericis
Copy link
Contributor

ericis commented Mar 21, 2024

@kurtaking I'll take a look. I am fairly certain we abandoned any OS-specific installs along with tarball packaging in favor of publishing the CLI to a private node registry and simply encouraging the use of NodeJS, project-specific dependencies that pin the version required and in exceptional cases to install globally (yarn global add ...) and/or use something like npx ... for single use. Last I looked, we were also looking at wrapping it in an OCI-compatible container to avoid specific runtime dependencies (e.g. node) and only require any OCI-compatible runner.

@kurtaking
Copy link

kurtaking commented Mar 22, 2024

@kurtaking I'll take a look. I am fairly certain we abandoned any OS-specific installs along with tarball packaging in favor of publishing the CLI to a private node registry and simply encouraging the use of NodeJS, project-specific dependencies that pin the version required and in exceptional cases to install globally (yarn global add ...) and/or use something like npx ... for single use. Last I looked, we were also looking at wrapping it in an OCI-compatible container to avoid specific runtime dependencies (e.g. node) and only require any OCI-compatible runner.

That's fair. I'm also not having issues with the yarn workspace and publishing the cli to our registry.

Running yarn oclif pack tarballs from the root of the workspace doesn't work because the cli is located at packages/cli.

Running yarn oclif pack tarballs in packages/cli isn't working because yarn has not resolved the 'example-pkg': 'workspace:^'-style dependencies.

It's been a pain to figure out with my current knowledge, but we can't abandon OS-specific installs right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Accepting PRs package managers Related to package manager support
Projects
None yet
Development

No branches or pull requests

4 participants