workers-sdk
workers-sdk copied to clipboard
fix: some peer dependency warnings
This fixes some peer dependency warning we were seeing when doing installs/builds
- Tests
- [ ] TODO (before merge)
- [ ] Included
- [x] Not necessary because: no functional changes
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- [ ] I don't know
- [ ] Required / Maybe required
- [x] Not required because: no functional changes
- Changeset (Changeset guidelines)
- [ ] TODO (before merge)
- [x] Included
- [ ] Not necessary because:
- Public documentation
- [ ] TODO (before merge)
- [ ] Cloudflare docs PR(s):
- [x] Not necessary because: no api/behaviour changes
🦋 Changeset detected
Latest commit: f785bc6db081969370767090f42fd8691ea0173a
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @cloudflare/vitest-pool-workers | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
@dario-piotrowicz are you familiar with the vitest-pool-workers codebase? I see the type errors here which seem legit, but I'm not sure what the fix should be. Is this something you can help with? No rush at all on this, or lemme know if someone else can help on it. If not, I'll be happy to dive in myself.
@dario-piotrowicz are you familiar with the vitest-pool-workers codebase? I see the type errors here which seem legit, but I'm not sure what the fix should be. Is this something you can help with? No rush at all on this, or lemme know if someone else can help on it. If not, I'll be happy to dive in myself.
@threepointone no sorry I am not really familiar with the vitest-pool-workers codebase, I am still totally happy to give it a look when I find the time if you want 🙂 (sometime next week I think)
also @petebacondarwin or @penalosa could be familiar with it
Vitest Pool Workers relies on a specific version of Vitest, which I think could cause issues such as this. When you pnpm install on this branch, you get:
WARN Issues with peer dependencies found
.
└─┬ @vitest/ui 1.6.0
└── ✕ unmet peer [email protected]: found 1.2.2
I believe the actual issue is to do with a conflict in the way Vitest extends the vite types. It adds this ambient declaration:
declare module 'vite' {
interface UserConfig {
/**
* Options for Vitest
*/
test?: VitestInlineConfig;
}
}
And for some reason, it's not picking this up, and is instead using the UserConfig from vite, which doesn't contain this test property.
@threepointone @petebacondarwin I think there is a solution to this, but I don't believe it's a long term solution.
This line is causing the Vite version to resolve to 5.3.1 in the root. And this version of vite is causing this type mismatch in my comment above.
The solution I have is to lock the vite version in vitest-pool-workers to 5.1.0:
"vite": "5.1.0",
(Note that in order to update this correctly, you will have to first checkout the pnpm-lock.yaml file from main and then pnpm install again, because there is some apparent hysteresis in the pnpm-lock.yaml, which means that the vitest-pool-workers package will still see the 5.3.1 version if you only pnpm install).
The other solution I would think is to upgrade Vitest in vitest-pool-workers, but I read that the version there is locked.
Referencing this here, because it's a different fix to the same issue:
https://github.com/cloudflare/next-on-pages/pull/811#discussion_r1652657818
@andyjessop feel free to take a call and own this PR?