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.

Sign up for GitHub

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

Jump to bottom

Feat: Adding option to specify the package manager version that you want #486

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feat: Adding option to specify the package manager version that you want #486

wants to merge 5 commits into from
+111 −1

Conversation

ramykl
Copy link
Contributor

@ramykl ramykl commented Sep 12, 2024

This will make it possible to install a specific version of a package manager through the cypress orb

@cypress-app-bot
Copy link
Collaborator

@ramykl ramykl changed the title Adding option to specify the package manager version that you want Feat: Adding option to specify the package manager version that you want Sep 18, 2024
@MikeMcC399 MikeMcC399 added the type: enhancement New feature or request label Oct 2, 2024
@deiga
Copy link
Contributor

deiga commented Oct 7, 2024

Could this also support using bun https://bun.sh/?

@MikeMcC399
Copy link
Collaborator

@deiga

Could this also support using bun https://bun.sh/?

The question of Bun support is a complex topic. The immediate answer to your question would be NO.

  • Add support for Bun cypress#28962 would need to be addressed, and there would be other issues as well. If you want to open a separate issue on the topic of Bun support, we could try and list the gating factors there, as far as they affect the Cypress CircleCI Orb.

@ramykl
Copy link
Contributor Author

ramykl commented Nov 4, 2024

@jennifer-shehane I think this will also fix the failing pnpm test you mentioned here #485 (comment) but I created a specific PR just for that fix as well #492

@MikeMcC399
Copy link
Collaborator

MikeMcC399 commented Nov 11, 2024 •
edited
Loading

@ramykl

Will you resolve the conflicts in this PR?

MikeMcC399
MikeMcC399 reviewed Nov 11, 2024
View reviewed changes
.circleci/test-deploy.yml
- Pnpm Example
- Pnpm Version Example
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pnpm Version Example is not defined

Are you using the VSCode CircleCI extension to catch errors in your syntax?

MikeMcC399
MikeMcC399 reviewed Nov 11, 2024
View reviewed changes
.circleci/test-deploy.yml
Comment on lines +91 to +93
post-install: "./check-yarn-version.sh 1.22.5 && npx cypress install"
package-manager: "yarn"
package-manager-version: "1.22.5"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yarn 1.22.5 is not really a useful version anymore.
For Yarn Classic v1, it's probably better to use Yarn 1.22.22

MikeMcC399
MikeMcC399 reviewed Nov 11, 2024
View reviewed changes
.circleci/test-deploy.yml
name: Yarn Version Example
working-directory: examples/yarn-install
cypress-cache-key: cypress-cache{{ arch }}-{{ checksum "examples/yarn-install/package.json" }}
post-install: "./check-yarn-version.sh 1.22.5 && npx cypress install"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you intend to support Yarn Modern Plug'n'Play then you can't use npx. It has to be yarn.

MikeMcC399
MikeMcC399 reviewed Nov 11, 2024
View reviewed changes
src/commands/install.yml
Comment on lines +83 to +85
- node/install:
install-yarn: true
yarn-version: << parameters.package-manager-version >>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://app.circleci.com/pipelines/github/ramykl/circleci-orb/38/workflows/47314291-67c5-462c-b813-e592ea2ec514/jobs/155

./src/commands/install.yml
83:11 warning wrong indentation: expected 8 but found 10 (indentation)

MikeMcC399
MikeMcC399 suggested changes Nov 11, 2024
View reviewed changes
Copy link
Collaborator

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments and suggestions

MikeMcC399
MikeMcC399 reviewed Nov 12, 2024
View reviewed changes
.circleci/test-deploy.yml
- cypress/run:
filters: *filters
name: Pnpm Example
working-directory: examples/pnpm-install
cypress-cache-key: cypress-cache{{ arch }}-{{ checksum "examples/pnpm-install/package.json" }}
post-install: "pnpm dlx cypress install"
package-manager: "pnpm"
- cypress/run:
filters: *filters
name: Pnpm Example
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate name - should probably be

Pnpm Version Example

@MikeMcC399
Copy link
Collaborator

Would you like to give some feedback about whether you want to continue with this PR?

It looks like the pnpm process is working now without needing to specify the pnpm version (it's just using the latest version by default).

@MikeMcC399
Copy link
Collaborator

Both pnpm and Yarn allow settting the package manager using corepack in current versions, so there seems to be some overlap between selecting the package manager version through the Orb and selecting it through the project configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeMcC399 MikeMcC399 MikeMcC399 requested changes

At least 1 approving review is required to merge this pull request.

Assignees

@ramykl ramykl

Labels
type: enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ramykl @cypress-app-bot @deiga @MikeMcC399