fix: read isLegacyEnv correctly

[threepointone]

This fixes the signature for isLegacyEnv() since it doesn't use args, and we fix reading legacy_env correctly when creating a draft worker when creating a secret.

[changeset-bot]

🦋 Changeset detected

Latest commit: c277a07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
wrangler	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.developers.workers.dev/runs/2173999288/npm-package-wrangler-807

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

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/807/npm-package-wrangler-807

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2173999288/npm-package-wrangler-807 dev path/to/script.js

[caass]

approving pending an answer on why the change u mentioned was made. makes sense otherwise

[threepointone]

yeah I'll let @JacobMGEvans have a look before I merge, thank you!

[petebacondarwin]

Can you double check that this is actually broken?

[JacobMGEvans]

This function is checking for validation of configuration

const isLegacyEnv =
    (args as { "legacy-env": boolean | undefined })["legacy-env"] ??
    rawConfig.legacy_env ??
    true;

This function seems to be checking against only the config which I think I missed when moving args to be generically passed to the validation check for the normalizeAndValidateConfig I would question if instead of removing the args from the param/argument of this helper, it should just look like the validator function; checking args then config to confirm which mode it is in.

function isLegacyEnv(args: unknown, config: Config): boolean {
  return config.legacy_env;
}

[threepointone]

It doesn't check args though, which is confusing. It's just a local helper, let's have it take only what it needs.

[JacobMGEvans]

It doesn't check args though, which is confusing. It's just a local helper, let's have it take only what it needs.

What I am saying is it should take both and use them, since the local helper is checking which mode it is in.
It is unclear to me why we would check args and config in the validator but not the helper checking what mode it is in locally which args is in scope of.

It is being resolved BY the validation and passed into CONFIG so the args is already resolved in the CLI parsing. (I wrote this refactor and forgot how it worked 😅 )

[threepointone]

Landing this since it's now a smaller fix, and Pete's on leave and I don't want to bother him more haha.

[petebacondarwin]

The original idea was to drop the isLegacyEnv() function altogether but we were going to do it in a follow up PR.