Skip to content

allow vitest to override invalid Wrangler assets config#9227

Merged
petebacondarwin merged 3 commits intomainfrom
pbd/vitest/allow-vitest-to-config-assets
May 16, 2025
Merged

allow vitest to override invalid Wrangler assets config#9227
petebacondarwin merged 3 commits intomainfrom
pbd/vitest/allow-vitest-to-config-assets

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

Fixes #9130

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: Vitest doesn't have e2e tests
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: bug fix
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: Vitest is not backported

@petebacondarwin petebacondarwin requested a review from a team as a code owner May 13, 2025 14:45
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk May 13, 2025
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is in its own commit, and just removes warnings from the build of the vitest-pool-workers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the crux of the fix, we pass in the assets configuration from the vitest config as an override.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 13, 2025

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/15051123524/npm-package-wrangler-9227
Prereleases for other packages:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-workers-bindings-extension-9227 -O ./cloudflare-workers-bindings-extension.0.0.0-v596934982.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v596934982.vsix

create-cloudflare:

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

@cloudflare/kv-asset-handler:

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

miniflare:

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

@cloudflare/pages-shared:

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

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-unenv-preset-9227

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-vite-plugin-9227

@cloudflare/vitest-pool-workers:

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

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-workers-editor-shared-9227

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-workers-shared-9227

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/15051123524/npm-package-cloudflare-workflows-shared-9227

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 13, 2025

🦋 Changeset detected

Latest commit: d8a9ee7

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

@petebacondarwin petebacondarwin force-pushed the pbd/vitest/allow-vitest-to-config-assets branch 2 times, most recently from 765058a to 593dc45 Compare May 13, 2025 15:03
Without this change you would see the following when building:

```
▲ [WARNING] The condition "types" here will never be used as it comes after both "import" and "require" [package.json]

    package.json:36:3:
      36 │       "types": "./dist/config/index.d.ts"
         ╵       ~~~~~~~

  The "import" condition comes earlier and will be used for all "import" statements:

    package.json:34:3:
      34 │       "import": "./dist/config/index.cjs",
         ╵       ~~~~~~~~

  The "require" condition comes earlier and will be used for all "require" calls:

    package.json:35:3:
      35 │       "require": "./dist/config/index.cjs",
```
@petebacondarwin petebacondarwin changed the title Pbd/vitest/allow vitest to config assets allow vitest to override invalid Wrangler assets config May 15, 2025
@petebacondarwin petebacondarwin force-pushed the pbd/vitest/allow-vitest-to-config-assets branch from 593dc45 to d8a9ee7 Compare May 15, 2025 17:13
Comment on lines +8 to +11
"test": "vitest --config vitest.workers.config.ts",
"test:ci": "run-script-os",
"test:ci:default": "vitest run --config vitest.workers.config.ts --reporter basic",
"test:ci:win32": "vitest run --config vitest.workers.config.ts --reporter basic --exclude test/sqlite-in-do.test.ts"
"test:ci:default": "vitest run --config vitest.workers.config.ts",
"test:ci:win32": "vitest run --config vitest.workers.config.ts --exclude test/sqlite-in-do.test.ts"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed --reporter basic because it is deprecated and has no effect.

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk May 16, 2025
@petebacondarwin petebacondarwin added this pull request to the merge queue May 16, 2025
Merged via the queue into main with commit ab8dab1 May 16, 2025
18 checks passed
@petebacondarwin petebacondarwin deleted the pbd/vitest/allow-vitest-to-config-assets branch May 16, 2025 09:51
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Vitest Integration mishandles ASSETS binding and prioritizes wrangler.jsonc over vitest.config.ts

3 participants