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

replace yarn with pnpm #6898

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

replace yarn with pnpm #6898

wants to merge 2 commits into from

Conversation

egasimus
Copy link

@egasimus egasimus commented Feb 18, 2025

Description

what's done

  • all instances of yarn.lock replaced with top-level pnpm-lock.yaml
    • postinstall in top-level package no longer needed
  • workspaces key in package.json replaced with pnpm-workspace.yml
  • changed how ng command is invoked
    • preserving max heap size flag
    • because pnpm installs shell wrappers in node_modules/.bin in place of js entrypoints
  • dependencies of each module specified using workspace:* instead of depending on top-level hoisting
    • some deps were only referenced in yarn.lock (scary!) brought them out of there and into the appropriate package.jsons
    • ⚠️ workspace:* version identifiers are pnpm-only
      • since packages are "private": true this doesnt matter in practice
      • publishing them to NPM (seamlessly and without the risk to bork local node_modules) may take some more specific version config but i assume for now it's a non-goal?

what's left

  • not sure how to provide process polyfill to client?
  • some top-level deps may now be redundant and need to be removed from top-level package.json
  • some workspace packages that i didn't see on my first pass may need dependencies/devDependencies provided
  • some peer dependency warnings that i might try to resolve while i'm at it
  • speculatively speaking, i'm curious to see whether the server can run on deno (feels like deno's built-in ts may tighten the feedback loop somewhat, and after 2.0 the node/npm compatibility is top notch)

Related issues

Has this been tested?

  • testing in progress. no new tests need to be written.
    acceptance criterion: pnpm i --frozen-lockfile && pnpm test pass

Copy link

socket-security bot commented Feb 18, 2025

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Obfuscated code npm/@browserstack/ai-sdk-node@1.5.17 ⚠︎

View full report↗︎

Next steps

What is obfuscated code?

Obfuscated files are intentionally packed to hide their behavior. This could be a sign of malware.

Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/@browserstack/ai-sdk-node@1.5.17

@egasimus egasimus changed the title wip: replace yarn with pnpm replace yarn with pnpm Feb 18, 2025
@egasimus egasimus marked this pull request as ready for review February 18, 2025 01:37

This comment was marked as outdated.

@Chocobozzz
Copy link
Owner

Chocobozzz commented Feb 18, 2025

Hi and thanks for the PR,

Unfortunately we don't plan to move away from yarn before v8 (planned at the end of 2025) because it requires a system dependency change + an update of our https://github.com/Chocobozzz/PeerTube/blob/develop/server/scripts/upgrade.sh#L82 script. We also have to migrate plugin management system to use pnpm instead of yarn.

@Chocobozzz Chocobozzz added the Status: Blocked ✋ Somehow, somewhere *else*, something has gone very wrong. Until they fix it we're stuck. label Feb 18, 2025
@egasimus
Copy link
Author

Acknowledged. If you think I can lend a hand with any of that, feel free to link any relevant tasks in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked ✋ Somehow, somewhere *else*, something has gone very wrong. Until they fix it we're stuck.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants