[wrangler] refactor: make experimental_readRawConfig and readConfig async #12031
Conversation
🦋 Changeset detectedLatest commit: 1ead9db The changes in this PR will be included in the next version bump. 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 |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
.changeset/async-read-config.md
Outdated
| "@cloudflare/vitest-pool-workers": minor | ||
| --- | ||
|
|
||
| Make `experimental_readRawConfig` and `unstable_getMiniflareWorkerOptions` async |
There was a problem hiding this comment.
While not technically a breaking change (because "experimental"), this will break people - i.e. Open Next.
Could me make it clearer in the changeset ?
Also unstable_readConfig should be mentioned there I think?
There was a problem hiding this comment.
Does open next have a sufficently locked down Wrangler version, or will this cause issues for users?
There was a problem hiding this comment.
it will cause issues, we use caret version everywhere
penalosa
force-pushed
the
penalosa/async-read-config
branch
from
8f1d3a1 to
96e97af
Compare
|
Blocking this until open next releases |
penalosa
force-pushed
the
penalosa/async-read-config
branch
from
fbd8007 to
3136a1d
Compare
Update all callers across wrangler, vitest-pool-workers, and vite-plugin-cloudflare to await the now-async config reading functions. This change prepares for future async config operations.
penalosa
force-pushed
the
penalosa/async-read-config
branch
from
3136a1d to
1ead9db
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| try { | ||
| const { rawConfig, configPath } = experimental_readRawConfig(args); | ||
| const { rawConfig, configPath } = await experimental_readRawConfig(args); |
There was a problem hiding this comment.
🟡 Awaiting synchronous experimental_readRawConfig instead of async version
The code is awaiting experimental_readRawConfig which is synchronous and returns a plain object, not a Promise.
Click to expand
The middleware function at packages/wrangler/src/index.ts:1781 uses:
const { rawConfig, configPath } = await experimental_readRawConfig(args);However, experimental_readRawConfig is defined as a synchronous function in packages/workers-utils/src/config/index.ts:137-154 that returns ReadRawConfigResult directly, not a Promise<ReadRawConfigResult>.
The async version is experimental_readRawConfigAsync (defined at packages/workers-utils/src/config/index.ts:164-169).
Impact: This will work at runtime because awaiting a non-Promise value just returns that value, but it's semantically incorrect and violates the intended API design. The code should either use experimental_readRawConfigAsync (and await it) or remove the await keyword.
Expected behavior: Use await experimental_readRawConfigAsync(args) to properly call the async version, which will support future code-based config files.
Recommendation: Change to await experimental_readRawConfigAsync(args) to use the proper async API
Was this helpful? React with 👍 or 👎 to provide feedback.
| const config = | ||
| def.behaviour?.provideConfig ?? true | ||
| ? readConfig(args, { | ||
| ? await readConfig(args, { |
There was a problem hiding this comment.
🟡 Awaiting synchronous readConfig function
The code is awaiting readConfig which is synchronous and returns Config, not Promise<Config>.
Click to expand
In packages/wrangler/src/core/register-yargs-command.ts:171, the code uses:
const config =
def.behaviour?.provideConfig ?? true
? await readConfig(args, {
hideWarnings: !(def.behaviour?.printConfigWarnings ?? true),
useRedirectIfAvailable:
def.behaviour?.useConfigRedirectIfAvailable,
})
: defaultWranglerConfig;However, readConfig is defined as a synchronous function in packages/wrangler/src/config/index.ts:105-111:
export function readConfig(
args: ReadConfigCommandArgs,
options: ReadConfigOptions = {}
): Config {
const raw = experimental_readRawConfig(args, options);
return convertRawConfigToConfig(args, options, raw);
}The async version is readConfigAsync (defined at packages/wrangler/src/config/index.ts:121-127).
Impact: This will work at runtime because awaiting a non-Promise value returns that value immediately, but it's semantically incorrect and defeats the purpose of the refactor to make config reading async.
Expected behavior: Use await readConfigAsync(args, {...}) to properly call the async version.
Recommendation: Change to await readConfigAsync(args, {...}) to use the proper async API
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Make
experimental_readRawConfigandunstable_getMiniflareWorkerOptionsasync in preparation for future async config operations.