chore: bump vitest to 1.5.0 and loosen version requirement for vitest-pool-workers

[Cherry]

What this PR solves / how to test

Bumps vitest to 1.5.0, to keep this inline with latest changes. All tests pass locally for me, and I tested this on a couple of my own projects without issue.

Closes #5615

Author has addressed the following

• Tests
  • TODO (before merge)
  • Included
  • Not necessary because:
• Changeset (Changeset guidelines)
  • TODO (before merge)
  • Included
  • Not necessary because:
• Public documentation
  • TODO (before merge)
  • Cloudflare docs PR(s): [Workers] bump vitest version cloudflare-docs#13706
  • Not necessary because:

[changeset-bot]

🦋 Changeset detected

Latest commit: 1261d4b

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

[github-actions]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-wrangler-5458

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5458/npm-package-wrangler-5458

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-wrangler-5458 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-create-cloudflare-5458 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-cloudflare-kv-asset-handler-5458

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-miniflare-5458

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-cloudflare-pages-shared-5458

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/9020843430/npm-package-cloudflare-vitest-pool-workers-5458

Note that these links will no longer work once the GitHub Actions artifact expires.

---

wrangler@3.55.0 includes the following runtime dependencies:

Package	Constraint	Resolved
miniflare	workspace:*	3.20240419.1
workerd	1.20240419.0	1.20240419.0
workerd --version	1.20240419.0	2024-04-19

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.23%. Comparing base (2789f26) to head (6c31516).
Report is 101 commits behind head on main.

❗ Current head 6c31516 differs from pull request most recent head 1261d4b. Consider uploading reports for the commit 1261d4b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5458      +/-   ##
==========================================
- Coverage   72.44%   72.23%   -0.22%     
==========================================
  Files         331      332       +1     
  Lines       17298    17249      -49     
  Branches     4422     4408      -14     
==========================================
- Hits        12532    12459      -73     
- Misses       4766     4790      +24     

see 21 files with indirect coverage changes

[Cherry]

I'm not sure what the issues are with the C3 E2E tests, but most everything else appears to be fine. If anyone has any insight here, that would be awesome.

[johtso]

@Cherry excited that this is happening! Do you know of any way I can take it for a spin in a local project? Not having much luck installing from github because of it being a monorepo. Tried using https://gitpkg.vercel.app but get  ERR_PNPM_WORKSPACE_PKG_NOT_FOUND  In ../..: "miniflare@workspace:*" is in the dependencies but no package named "miniflare" is present in the workspace

[Cherry]

@johtso If you add:

"@cloudflare/vitest-pool-workers": "https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8561120366/npm-package-cloudflare-vitest-pool-workers-5458"

Or just run:
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8561120366/npm-package-cloudflare-vitest-pool-workers-5458 -D

to your package.json dev dependencies, you should be able to test with this.

[jculvey]

C3 changes lgtm

[johtso]

@johtso If you add:

"@cloudflare/vitest-pool-workers": "https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8561120366/npm-package-cloudflare-vitest-pool-workers-5458"

Or just run: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8561120366/npm-package-cloudflare-vitest-pool-workers-5458 -D

to your package.json dev dependencies, you should be able to test with this.

This is exactly what I was looking for, thank you so much!

[jculvey]

I'm not sure what the issues are with the C3 E2E tests, but most everything else appears to be fine. If anyone has any insight here, that would be awesome.

The issue is that the C3 E2E tests run actual deployments and need credentials to run properly. When PRs are made from forks, the C3_TEST_CLOUDFLARE_ACCOUNT_ID and C3_TEST_CLOUDFLARE_API_TOKEN secrets are unset which makes the tests fail.

I'd be happy to run the E2Es manually to validate if needed.

[Cherry]

Updated this for 1.5.0. Tested and all seems to be working well.

[johtso]

Also maybe consider warning the developer rather than throwing an error if they choose to install a newer version of vitest than this packages has been tested with?

[petebacondarwin]

Also maybe consider warning the developer rather than throwing an error if they choose to install a newer version of vitest than this packages has been tested with?

I believe most package managers will only warn (not error) if you have installed a peer dependency that is outside the range given.

[johtso]

I believe most package managers will only warn (not error) if you have installed a peer dependency that is outside the range given.

The package currently explicitly asserts that the installed vitest version matches the dependency version:

workers-sdk/packages/vitest-pool-workers/src/pool/index.ts

Lines 753 to 786 in 21878f5

	function assertCompatibleVitestVersion(ctx: Vitest) {
	// Some package managers don't enforce `peerDependencies` requirements,
	// so add a runtime sanity check to ensure things don't break in strange ways.
	const poolPkgJson = getPackageJson(__dirname);
	const vitestPkgJson = getPackageJson(ctx.distPath);
	assert(
	poolPkgJson !== undefined,
	"Expected to find `package.json` for `@cloudflare/vitest-pool-workers`"
	);
	assert(
	vitestPkgJson !== undefined,
	"Expected to find `package.json` for `vitest`"
	);
	const expectedVitestVersion = poolPkgJson.peerDependencies?.vitest;
	const actualVitestVersion = vitestPkgJson.version;
	assert(
	expectedVitestVersion !== undefined,
	"Expected to find `@cloudflare/vitest-pool-workers`'s `vitest` version constraint"
	);
	assert(
	actualVitestVersion !== undefined,
	"Expected to find `vitest`'s version"
	);
	if (expectedVitestVersion !== actualVitestVersion) {
	const message = [
	`You're running \`vitest@${actualVitestVersion}\`, but this version of \`@cloudflare/vitest-pool-workers\` only supports \`vitest@${expectedVitestVersion}\`.`,
	"`@cloudflare/vitest-pool-workers` currently depends on internal Vitest APIs that are not protected by semantic-versioning guarantees.",
	`Please install \`vitest@${expectedVitestVersion}\` to continue using \`@cloudflare/vitest-pool-workers\`.`,
	].join("\n");
	throw new Error(message);
	}
	}

[petebacondarwin]

Cripes! OK. So I think we probably should relax that a bit. I think a warning should be enough.

[Cherry]

Done! Here's what the warning now looks like:

Very happy to update this if someone has any better wording.

[petebacondarwin]

TIL - hyphenated ranges!

[Cherry]

Same! >=1.3.0 || <= 1.5.x didn't work as I expected, because 1.2.0 satisfied this, as it's < 1.5.x, but hyphenated worked great!

[petebacondarwin]

Why is there a .js on the end of this module specifier?

[Cherry]

I'm not entirely sure, but I assume it's just due to how semver is built and published in a very CJS fashion?

Without it, I got an error like this:

Error: Cannot find module 'E:\xxx\node_modules\semver\functions\satisfies' imported from E:\xxx\node_modules\@cloudflare\vitest-pool-workers\dist\pool\index.mjs
Did you mean to import semver/functions/satisfies.js?
 ❯ finalizeResolution node:internal/modules/esm/resolve:264:11
 ❯ moduleResolve node:internal/modules/esm/resolve:917:10
 ❯ defaultResolve node:internal/modules/esm/resolve:1130:11
 ❯ ModuleLoader.defaultResolve node:internal/modules/esm/loader:396:12
 ❯ ModuleLoader.resolve node:internal/modules/esm/loader:365:25
 ❯ ModuleLoader.getModuleJob node:internal/modules/esm/loader:240:38
 ❯ ModuleWrap.<anonymous> node:internal/modules/esm/module_job:85:39
 ❯ link node:internal/modules/esm/module_job:84:36

And when swapping over to semver/functions/satisfies.js everything was happy, so I didn't dig any further.

[Cherry]

Another option would be to just import semver from 'semver' and then using semver.satisfies, if that's preferred. Let me know.

[DaniFoldi]

Hey @petebacondarwin,
I see that you have been rebasing this branch and looking at e2e tests.

Vitest 1.6.0 was released yesterday, and there are some fixes in recent versions that we depend on. Do you think you'll have a chance to land this soon? Many people on Discord are also asking about updates, and a link to this PR is the only answer we can give, unfortunately.

Ps. happy to take a look at 1.6.0 to see if it breaks something, but even 1.5.x would be great.

[Cherry]

I tried to bump to v1.6.0 but hit a roadblock that I believe to be caused by this change:

vitest-dev/vitest@d8304bb#diff-2a750ef38c91574e0278bc98b00b991a28ad7e03949fd6574bf8bb049615781cR33

After updating, all tests were failing with Error: readFileSync() is not yet implemented in Workers.

I believe this is due to readFileSync now actually being used in the vitest runner, which has since been stubbed here to error as per: https://github.com/cloudflare/workers-sdk/blob/027f971975a48a564603275f3583d21e9d053229/packages/vitest-pool-workers/src/worker/lib/node/fs.ts

I'm not familiar enough with vitest internals or this code to dive much deeper in stubbing/patching this, but using 1.5.x works great and would be good to at least get that merged in soon until someone else can take a look at 1.6.x.

[petebacondarwin]

I'm trying to land it but the tests are not playing along. I'll try to dig in and sort it this coming week

[Cherry]

Anything I can do to help here @petebacondarwin? It seems the standard tests keep passing fine, and it's really just the C3 ones which are failing?

[petebacondarwin]

Let's give it one more go and then I'll dig in a bit more deeply.
It is possible to run the C3 e2e tests locally, which I will try if this doesn't go well

[petebacondarwin]

Looks like the issues are npm ones:

npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: tmp-e2e-c3e2da7571-hello-world-js@0.0.0
npm ERR! Found: vitest@1.5.0
npm ERR! node_modules/vitest
npm ERR!   dev vitest@"1.5.0" from the root project
npm ERR!   peer vitest@"1.5.0" from @vitest/browser@1.5.0
npm ERR!   node_modules/@vitest/browser
npm ERR!     peerOptional @vitest/browser@"1.5.0" from vitest@1.5.0
npm ERR!   1 more (@vitest/ui)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer vitest@"1.3.0" from @cloudflare/vitest-pool-workers@0.1.19
npm ERR! node_modules/@cloudflare/vitest-pool-workers
npm ERR!   dev @cloudflare/vitest-pool-workers@"^0.1.0" from the root project

[petebacondarwin]

I think we need to bump this to "^0.2.7":

workers-sdk/packages/create-cloudflare/templates/hello-world/js/package.json

Line 14 in f673c66

"@cloudflare/vitest-pool-workers": "^0.1.0"

and also for the ts one.

[Cherry]

Hmm, interesting. Wouldn't it need to pin to the workspace version with the updated peer deps, since 0.2.7 will also be pinned to the old vitest 1.3.0?

[petebacondarwin]

Hmm, interesting. Wouldn't it need to pin to the workspace version with the updated peer deps, since 0.2.7 will also be pinned to the old vitest 1.3.0?

I don't think we can use the workspace because these are templates that get used outside of the workers-sdk repo. I.e. when running C3 externally. So it has to point to a publicly deployed version.

I think it is a bit of a chicken and egg thing? We need the C3 e2e tests to pass before we release the new version and we need the new version released to run the C3 e2e tests.

Perhaps we just don't update these C3 templates to vitest 1.5.0 for the time being?

[Cherry]

Perhaps we just don't update these C3 templates to vitest 1.5.0 for the time being?

That might be the best solution here, yeah. And then I can make a followup PR once this lands in a release, updating the C3 templates afterwards.

Thanks for digging in here.

[RamIdeas]

I think it is a bit of a chicken and egg thing? We need the C3 e2e tests to pass before we release the new version and we need the new version released to run the C3 e2e tests.

Can an override in the top-level package.json help here? The template can point to the released public version (for anyone cloning the template) and the e2e tests will respect the override and use the workspace version

[petebacondarwin]

I think it is a bit of a chicken and egg thing? We need the C3 e2e tests to pass before we release the new version and we need the new version released to run the C3 e2e tests.

Can an override in the top-level package.json help here? The template can point to the released public version (for anyone cloning the template) and the e2e tests will respect the override and use the workspace version

Yeah I thought about that. But I think that is overkill for this situation. These templates are not really part of the key infrastructure that we want to test against local versions of stuff. They are just for external users who will always be using the publicly deployed version of c3. So I don't think it is worth the extra hassle of switching out the versions just for our C3 e2e tests.