Skip to content

Commit

Permalink
fix: ensure the correct worker name is published in legacy environmen…
Browse files Browse the repository at this point in the history
…ts (#719)

When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See #680 (comment)
  • Loading branch information
petebacondarwin committed Mar 28, 2022
1 parent 18d09c7 commit 6503ace
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 12 deletions.
18 changes: 18 additions & 0 deletions .changeset/beige-olives-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"wrangler": patch
---

fix: ensure the correct worker name is published in legacy environments

When a developer uses `--env` to specify an environment name, the Worker name should
be computed from the top-level Worker name and the environment name.

When the given environment name does not match those in the wrangler.toml, we error.
But if no environments have been specified in the wrangler.toml, at all, then we only
log a warning and continue.

In this second case, we were reusing the top-level environment, which did not have the
correct legacy environment fields set, such as the name. Now we ensure that such an
environment is created as needed.

See https://github.com/cloudflare/wrangler2/pull/680#issuecomment-1080407556
62 changes: 53 additions & 9 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe("publish", () => {
expect(std.warn).toMatchInlineSnapshot(`""`);
});

it("appends the environment name when provided", async () => {
it("appends the environment name when provided, and there is associated config", async () => {
writeWranglerToml({ env: { "some-env": {} } });
writeWorkerSource();
mockSubDomainRequest();
Expand All @@ -90,14 +90,58 @@ describe("publish", () => {
expect(std.warn).toMatchInlineSnapshot(`""`);
});

it("should throw an error w/ helpful message when using --env --name", async () => {
writeWranglerToml({ env: { "some-env": {} } });
it("appends the environment name when provided (with a warning), if there are no configured environments", async () => {
writeWranglerToml({});
writeWorkerSource();
mockSubDomainRequest();
mockUploadWorkerRequest({
env: "some-env",
legacyEnv: true,
});
await runWrangler("publish index.js --env some-env --legacy-env true");
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name-some-env (TIMINGS)
Published test-name-some-env (TIMINGS)
test-name-some-env.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`
"Processing wrangler.toml configuration:
- No environment found in configuration with name \\"some-env\\".
Before using \`--env=some-env\` there should be an equivalent environment section in the configuration.
Consider adding an environment configuration section to the wrangler.toml file:
\`\`\`
[env.some-env]
\`\`\`
"
`);
});

it("should throw an error when an environment name when provided, which doesn't match those in the config", async () => {
writeWranglerToml({ env: { "other-env": {} } });
writeWorkerSource();
mockSubDomainRequest();
await expect(
runWrangler("publish index.js --env some-env --legacy-env true")
).rejects.toThrowErrorMatchingInlineSnapshot(`
"Processing wrangler.toml configuration:
- No environment found in configuration with name \\"some-env\\".
Before using \`--env=some-env\` there should be an equivalent environment section in the configuration.
The available configured environment names are: [\\"other-env\\"]
Consider adding an environment configuration section to the wrangler.toml file:
\`\`\`
[env.some-env]
\`\`\`
"
`);
});

it("should throw an error w/ helpful message when using --env --name", async () => {
writeWranglerToml({ env: { "some-env": {} } });
writeWorkerSource();
mockSubDomainRequest();
await runWrangler(
"publish index.js --name voyager --env some-env --legacy-env true"
).catch((err) =>
Expand Down Expand Up @@ -2801,12 +2845,12 @@ export default{

await expect(runWrangler("publish index.js")).rejects
.toThrowErrorMatchingInlineSnapshot(`
"You seem to be trying to use Durable Objects in a Worker written with Service Worker syntax.
You can use Durable Objects defined in other Workers by specifying a \`script_name\` in your wrangler.toml, where \`script_name\` is the name of the Worker that implements that Durable Object. For example:
{ name = EXAMPLE_DO_BINDING, class_name = ExampleDurableObject } ==> { name = EXAMPLE_DO_BINDING, class_name = ExampleDurableObject, script_name = example-do-binding-worker }
Alternatively, migrate your worker to ES Module syntax to implement a Durable Object in this Worker:
https://developers.cloudflare.com/workers/learning/migrating-to-module-workers/"
`);
"You seem to be trying to use Durable Objects in a Worker written with Service Worker syntax.
You can use Durable Objects defined in other Workers by specifying a \`script_name\` in your wrangler.toml, where \`script_name\` is the name of the Worker that implements that Durable Object. For example:
{ name = EXAMPLE_DO_BINDING, class_name = ExampleDurableObject } ==> { name = EXAMPLE_DO_BINDING, class_name = ExampleDurableObject, script_name = example-do-binding-worker }
Alternatively, migrate your worker to ES Module syntax to implement a Durable Object in this Worker:
https://developers.cloudflare.com/workers/learning/migrating-to-module-workers/"
`);
});
});

Expand Down
18 changes: 15 additions & 3 deletions packages/wrangler/src/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ export function normalizeAndValidateConfig(

let activeEnv = topLevelEnv;
if (envName !== undefined) {
const envDiagnostics = new Diagnostics(
`"env.${envName}" environment configuration`
);
const rawEnv = rawConfig.env?.[envName];
if (rawEnv !== undefined) {
const envDiagnostics = new Diagnostics(
`"env.${envName}" environment configuration`
);
activeEnv = normalizeAndValidateEnvironment(
envDiagnostics,
configPath,
Expand All @@ -124,6 +124,18 @@ export function normalizeAndValidateConfig(
);
diagnostics.addChild(envDiagnostics);
} else {
// An environment was specified, but no configuration for it was found.
// To cover any legacy environment cases, where the `envName` is used,
// Let's create a fake active environment with the specified `envName`.
activeEnv = normalizeAndValidateEnvironment(
envDiagnostics,
configPath,
{},
envName,
topLevelEnv,
isLegacyEnv,
rawConfig
);
const envNames = rawConfig.env
? `The available configured environment names are: ${JSON.stringify(
Object.keys(rawConfig.env)
Expand Down

0 comments on commit 6503ace

Please sign in to comment.