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

fix(deps): update open dependency to 10.1.0 #18349

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

niklasmh
Copy link

@niklasmh niklasmh commented Oct 14, 2024

Description

This is a fix for issue #14292 (converted to discussion #14293). I still get this error when running Vite in PowerShell (on Windows 10), which means the bug fix from the open dependency is not in Vite yet:

VITE v5.4.8  ready in 317 ms

  ➜  Local:   http://localhost:3001/
  ➜  Network: use --host to expose
  ➜  press h + enter to show help
Error: spawn undefined\System32\WindowsPowerShell\v1.0\powershell ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

In order to fix this, I upgraded the open package to 10.1.0 in packages/vite/package.json (the bug was fixed in version 10.0.1). I did not find any similar PRs to upgrade this package (not even in version v6.0.0-beta.2), so I made this.

The only breaking change going from ^8.4.2 to ^10.1.0 is that node should be >=18. Vite also requires node>=18, so this should not break anything.

Copy link

stackblitz bot commented Oct 14, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@niklasmh niklasmh changed the title Update open dependency to 10.1.0 chore: Update open dependency to 10.1.0 Oct 14, 2024
@niklasmh niklasmh changed the title chore: Update open dependency to 10.1.0 chore: update open dependency to 10.1.0 Oct 14, 2024
@niklasmh
Copy link
Author

I am not sure how to update the pnpm-lock.yaml file properly using StackBlitz. I tried doing the minimal changes manually since the pnpm on StackBlitz enforces me to change the format of the lock file adding too many changes.

@bluwy
Copy link
Member

bluwy commented Oct 14, 2024

We didn't update in the past due to increased package size (#15421), but seems like recently it shrunk down so we could probably upgrade it again now.

@niklasmh
Copy link
Author

We didn't update in the past due to increased package size (#15421), but seems like recently it shrunk down so we could probably upgrade it again now.

@bluwy Thanks for the explanation. That is good news. I tried fixing the issues in the lock file manually, but it seems like I don't have the right tools now. If anyone with the right setup could update the package that would be awesome.

@niklasmh
Copy link
Author

Seems like the dependencies are fixed now. I did another fix with how the Options type from the open package was imported that worked. Now the tests are failing. Not sure how to begin here.

@sapphi-red sapphi-red added dependencies Pull requests that update a dependency file p3-minor-bug An edge case that only affects very specific usage (priority) labels Oct 15, 2024
@sapphi-red sapphi-red changed the title chore: update open dependency to 10.1.0 fix(deps): update open dependency to 10.1.0 Oct 15, 2024
sapphi-red
sapphi-red previously approved these changes Oct 15, 2024
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

Thanks!

packages/vite/rollup.config.ts Show resolved Hide resolved
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Nice to see the shims removed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants