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

🐛 BUG: vitest-pool-workers: Pages project with pages_build_output_dir in wrangler.toml fails #5768

Closed
osaton opened this issue May 6, 2024 · 4 comments · Fixed by #6911 or #6907
Assignees
Labels
bug Something that isn't working pages Relating to Pages pages-config wrangler.toml support for Pages related issues vitest Relating to the Workers Vitest integration

Comments

@osaton
Copy link

osaton commented May 6, 2024

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.35.1

What version of Node are you using?

20.11.0

What operating system and version are you using?

Mac Sonoma 14.4

Describe the Bug

Observed behavior

Test process exits with error "Pages doesn't currently support JSON formatted config /my-project/wrangler.toml. Please use wrangler.toml instead." when run in a pages project with wrangler.toml containing pages_build_output_dir

Expected behavior

Should run tests normally

Please provide a link to a minimal reproduction

https://github.com/osaton/temp-black-waterfall-44dc

Please provide any relevant error logs

❯  readConfig node_modules/wrangler/wrangler-dist/cli.js:158419:11
❯ Module.unstable_getMiniflareWorkerOptions node_modules/wrangler/wrangler-dist/cli.js:206586:18
❯ parseCustomPoolOptions node_modules/@cloudflare/vitest-pool-workers/dist/pool/index.mjs:212:54
❯ parseProjectOptions node_modules/@cloudflare/vitest-pool-workers/dist/pool/index.mjs:253:12
❯ Object.runTests node_modules/@cloudflare/vitest-pool-workers/dist/pool/index.mjs:1410:22

First unstable_getMiniflareWorkerOptions passes experimentalJsonConfig: true to readConfig which throws at packages/wrangler/src/config/index.ts:100 because of experimantalJsonConfig check:

if (
	isPagesConfigFile &&
	(configPath?.endsWith("json") || args.experimentalJsonConfig)
) {
	throw new UserError(
		`Pages doesn't currently support JSON formatted config \`${
			configPath ?? "wrangler.json"
		}\`. Please use wrangler.toml instead.`
	);
}

Which makes me think if the || args.experimentalJsonConfig part is even needed here? we have already parsed the config based on the file extension, so we know that the config is not in JSON format, so maybe isPagesConfigFile && configPath?.endsWith("json") would be sufficient here?

@osaton osaton added the bug Something that isn't working label May 6, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk May 6, 2024
@ericmatthys
Copy link

I came across this bug trying to get the Vitest integration working with Pages Functions and Wasm. Patching the experimentalJsonConfig option to false when unstable_getMiniflareWorkerOptions is called was enough to unblock me for now, but that is just a temporary hack of course.

@vladinator1000
Copy link

vladinator1000 commented Jun 4, 2024

I see the same error with this config

import { defineWorkersConfig } from '@cloudflare/vitest-pool-workers/config'

export default defineWorkersConfig({
  test: {
    globals: true,
    include: ['./server/**/*.test.{ts,tsx}'],
    poolOptions: {
      workers: {
        miniflare: {},
        wrangler: { configPath: './wrangler.toml' },
      },
    },
  },
})

It starts working when I comment out # pages_build_output_dir = "build/client" from wrangler.toml
"@cloudflare/vitest-pool-workers": "^0.4.1"

@nicolaspapp
Copy link

Had the same issue, which was also fixed with @osaton's suggestion.

@penalosa penalosa moved this from Untriaged to Backlog in workers-sdk Jun 17, 2024
@petebacondarwin petebacondarwin added pages Relating to Pages vitest Relating to the Workers Vitest integration pages-config wrangler.toml support for Pages related issues labels Aug 5, 2024
@RamIdeas
Copy link
Contributor

RamIdeas commented Sep 2, 2024

Proposed fix: Add option to vitest-pool-worker config to experimentalJsonConfig to allow the users to decide whether to enable json config files explicitly (default should be false, even though it is currently true)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working pages Relating to Pages pages-config wrangler.toml support for Pages related issues vitest Relating to the Workers Vitest integration
Projects
Archived in project
7 participants