From 7c7062848bf50caf1fafed704e155d120683c12c Mon Sep 17 00:00:00 2001 From: Daniel Rivas <1887507+danielrs@users.noreply.github.com> Date: Tue, 26 Aug 2025 12:20:27 -0500 Subject: [PATCH 1/5] feat: extract subdomain deploy into its own function Should make the logic easier to understand. --- packages/wrangler/src/triggers/deploy.ts | 141 +++++++++++++---------- 1 file changed, 81 insertions(+), 60 deletions(-) diff --git a/packages/wrangler/src/triggers/deploy.ts b/packages/wrangler/src/triggers/deploy.ts index 79c12c4459ec..f9dbcbdb7c0d 100644 --- a/packages/wrangler/src/triggers/deploy.ts +++ b/packages/wrangler/src/triggers/deploy.ts @@ -60,9 +60,6 @@ export default async function triggersDeploy( } } - // deployToWorkersDev defaults to true only if there aren't any routes defined - const deployToWorkersDev = getResolvedWorkersDev(config.workers_dev, routes); - if (!scriptName) { throw new UserError( 'You need to provide a name when uploading a Worker Version. Either pass it as a cli arg with `--name ` or in your config file as `name = ""`', @@ -79,14 +76,6 @@ export default async function triggersDeploy( ? `/accounts/${accountId}/workers/services/${scriptName}/environments/${envName}` : `/accounts/${accountId}/workers/scripts/${scriptName}`; - const { - enabled: available_on_subdomain, - previews_enabled: previews_available_on_subdomain, - } = await fetchResult<{ - enabled: boolean; - previews_enabled: boolean; - }>(config, `${workerUrl}/subdomain`); - if (!props.dryRun) { await ensureQueuesExistByConfig(config); } @@ -103,55 +92,17 @@ export default async function triggersDeploy( const uploadMs = Date.now() - start; const deployments: Promise[] = []; - const deploymentInSync = deployToWorkersDev === available_on_subdomain; - const previewsInSync = - config.preview_urls === previews_available_on_subdomain; - - if (deployToWorkersDev) { - // Deploy to a subdomain of `workers.dev` - const userSubdomain = await getWorkersDevSubdomain( - config, - accountId, - config.configPath - ); - - const deploymentURL = - props.legacyEnv || !props.env - ? `${scriptName}.${userSubdomain}` - : `${envName}.${scriptName}.${userSubdomain}`; - - if (deploymentInSync && previewsInSync) { - deployments.push(Promise.resolve([deploymentURL])); - } else { - // Enable the `workers.dev` subdomain. - deployments.push( - fetchResult(config, `${workerUrl}/subdomain`, { - method: "POST", - body: JSON.stringify({ - enabled: true, - previews_enabled: config.preview_urls, - }), - headers: { - "Content-Type": "application/json", - }, - }).then(() => [deploymentURL]) - ); - } - } - if (!deployToWorkersDev && (!deploymentInSync || !previewsInSync)) { - // Disable the workers.dev deployment - await fetchResult(config, `${workerUrl}/subdomain`, { - method: "POST", - body: JSON.stringify({ - enabled: false, - previews_enabled: config.preview_urls, - }), - headers: { - "Content-Type": "application/json", - }, - }); - } - if (!deployToWorkersDev && deploymentInSync && routes.length !== 0) { + const { wantWorkersDev, workersDevInSync } = await subdomainDeploy( + props, + accountId, + scriptName, + envName, + workerUrl, + routes, + deployments + ); + + if (!wantWorkersDev && workersDevInSync && routes.length !== 0) { // TODO is this true? How does last subdomain status affect route confict?? // Why would we only need to validate route conflicts if didn't need to // disable the subdomain deployment? @@ -349,3 +300,73 @@ export default async function triggersDeploy( logger.log("No deploy targets for", workerName, formatTime(deployMs)); } } + +async function subdomainDeploy( + props: Props, + accountId: string, + scriptName: string, + envName: string, + workerUrl: string, + routes: Route[], + deployments: Array> +) { + const { config } = props; + + // Get desired subdomain enablement status. + + const defaultWorkersDev = routes.length === 0; + const wantWorkersDev = config.workers_dev ?? defaultWorkersDev; + const wantPreviews = config.preview_urls ?? false; + + // Get current subdomain enablement status. + + const { enabled: currWorkersDev, previews_enabled: currPreviews } = + await fetchResult<{ + enabled: boolean; + previews_enabled: boolean; + }>(config, `${workerUrl}/subdomain`); + + const workersDevInSync = wantWorkersDev === currWorkersDev; + const previewsInSync = wantPreviews === currPreviews; + const allInSync = [workersDevInSync, previewsInSync].every((v) => v); + + // workers.dev URL is only set if we want to deploy to workers.dev. + + let workersDevURL: string | undefined; + if (wantWorkersDev) { + const userSubdomain = await getWorkersDevSubdomain( + config, + accountId, + config.configPath + ); + workersDevURL = + props.legacyEnv || !props.env + ? `${scriptName}.${userSubdomain}` + : `${envName}.${scriptName}.${userSubdomain}`; + } + + // Update subdomain enablement status if needed. + + if (!allInSync) { + await fetchResult(config, `${workerUrl}/subdomain`, { + method: "POST", + body: JSON.stringify({ + enabled: wantWorkersDev, + previews_enabled: wantPreviews, + }), + headers: { + "Content-Type": "application/json", + }, + }); + } + + if (workersDevURL) { + deployments.push(Promise.resolve([workersDevURL])); + } + return { + wantWorkersDev, + wantPreviews, + workersDevInSync, + previewsInSync, + }; +} From 00d71e077d582f1226d88b9c4631a326d0c03b52 Mon Sep 17 00:00:00 2001 From: Daniel Rivas <1887507+danielrs@users.noreply.github.com> Date: Tue, 26 Aug 2025 12:57:50 -0500 Subject: [PATCH 2/5] fix: config preview_urls defaults to undefined (falsy) instead of true We want preview_urls to be opt-in, rather than opt-out. For this reason its default value will be falsy. --- .changeset/ready-banks-repair.md | 8 +++++ .../__tests__/config/configuration.test.ts | 2 +- .../wrangler/src/__tests__/deploy.test.ts | 23 +++++++++----- .../src/__tests__/deploy/config-diffs.test.ts | 26 ++++++---------- .../__tests__/helpers/mock-upload-worker.ts | 29 ++++++++++++++++-- .../helpers/mock-workers-subdomain.ts | 4 +-- .../helpers/write-wrangler-config.ts | 6 +++- packages/wrangler/src/config/config.ts | 2 +- packages/wrangler/src/config/environment.ts | 2 +- packages/wrangler/src/config/validation.ts | 2 +- packages/wrangler/src/deploy/config-diffs.ts | 14 +++++---- packages/wrangler/src/triggers/deploy.ts | 30 +++++++++++-------- .../src/utils/download-worker-config.ts | 10 ++++--- 13 files changed, 102 insertions(+), 56 deletions(-) create mode 100644 .changeset/ready-banks-repair.md diff --git a/.changeset/ready-banks-repair.md b/.changeset/ready-banks-repair.md new file mode 100644 index 000000000000..36bb912cde04 --- /dev/null +++ b/.changeset/ready-banks-repair.md @@ -0,0 +1,8 @@ +--- +"wrangler": minor +--- + +Beta feature preview_urls is now disabled by default. + +This change makes preview_urls disabled by default when it's not provided, making +the feature opt-in instead of opt-out. diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 5ff8c5ee0471..3e1dfa4b9e63 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -130,7 +130,7 @@ describe("normalizeAndValidateConfig()", () => { wasm_modules: undefined, data_blobs: undefined, workers_dev: undefined, - preview_urls: true, + preview_urls: undefined, zone_id: undefined, no_bundle: undefined, minify: undefined, diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 6cc09122858c..fc6a7ad4a017 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -1898,7 +1898,7 @@ Update them to point to this script instead?`, }); await mockAUSRequest([]); mockSubDomainRequest(); - mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: true }); + mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: false }); mockUploadWorkerRequest({ expectedAssets: { jwt: "<>", @@ -1983,7 +1983,7 @@ Update them to point to this script instead?`, }); await mockAUSRequest([]); mockSubDomainRequest(); - mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: true }); + mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: false }); mockUploadWorkerRequest({ expectedAssets: { jwt: "<>", @@ -2046,7 +2046,7 @@ Update them to point to this script instead?`, writeWorkerSource(); await mockAUSRequest([]); mockSubDomainRequest(); - mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: true }); + mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: false }); mockUploadWorkerRequest({ expectedAssets: { jwt: "<>", @@ -2116,7 +2116,7 @@ Update them to point to this script instead?`, }); await mockAUSRequest([]); mockSubDomainRequest(); - mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: true }); + mockUpdateWorkerSubdomain({ enabled: false, previews_enabled: false }); mockUploadWorkerRequest({ expectedAssets: { jwt: "<>", @@ -12987,7 +12987,10 @@ export default{ ]); mockGetServiceRoutes("test-name", []); mockGetServiceCustomDomainRecords([]); - mockGetServiceSubDomainData("test-name", { enabled: true }); + mockGetServiceSubDomainData("test-name", { + enabled: true, + previews_enabled: false, + }); mockGetServiceSchedules("test-name", { schedules: [] }); mockGetServiceMetadata("test-name", { created_on: "2025-08-07T09:34:47.846308Z", @@ -13012,7 +13015,7 @@ export default{ "▲ [WARNING] The local configuration being used (generated from your local configuration file) differs from the remote configuration of your Worker set via the Cloudflare Dashboard: \\"workers_dev\\": true, - \\"preview_urls\\": true, + \\"preview_urls\\": false, \\"vars\\": { - \\"MY_VAR\\": \\"abc\\" + \\"MY_VAR\\": 123 @@ -13049,7 +13052,10 @@ export default{ ]); mockGetServiceRoutes("test-name", []); mockGetServiceCustomDomainRecords([]); - mockGetServiceSubDomainData("test-name", { enabled: true }); + mockGetServiceSubDomainData("test-name", { + enabled: true, + previews_enabled: false, + }); mockGetServiceSchedules("test-name", { schedules: [] }); mockGetServiceMetadata("test-name", { created_on: "2025-08-07T09:34:47.846308Z", @@ -13077,7 +13083,7 @@ export default{ "▲ [WARNING] The local configuration being used (generated from your local configuration file) differs from the remote configuration of your Worker set via the Cloudflare Dashboard: \\"workers_dev\\": true, - \\"preview_urls\\": true, + \\"preview_urls\\": false, \\"vars\\": { - \\"MY_VAR\\": \\"abc\\" + \\"MY_VAR\\": \\"this is a toml file\\" @@ -13925,6 +13931,7 @@ function mockGetServiceSubDomainData( serviceName: string, data: { enabled: boolean; + previews_enabled: boolean; } ) { msw.use( diff --git a/packages/wrangler/src/__tests__/deploy/config-diffs.test.ts b/packages/wrangler/src/__tests__/deploy/config-diffs.test.ts index 2926b8af2566..9a97a009ddc1 100644 --- a/packages/wrangler/src/__tests__/deploy/config-diffs.test.ts +++ b/packages/wrangler/src/__tests__/deploy/config-diffs.test.ts @@ -8,6 +8,7 @@ describe("getRemoteConfigsDiff", () => { name: "silent-firefly-dbe3", main: "/tmp/src/index.js", workers_dev: true, + preview_urls: false, compatibility_date: "2025-07-08", compatibility_flags: undefined, placement: undefined, @@ -38,6 +39,7 @@ describe("getRemoteConfigsDiff", () => { name: "silent-firefly-dbe3", main: "/tmp/src/index.js", workers_dev: true, + preview_urls: false, compatibility_date: "2025-07-08", compatibility_flags: undefined, placement: undefined, @@ -75,6 +77,7 @@ describe("getRemoteConfigsDiff", () => { name: "silent-firefly-dbe3", main: "/tmp/src/index.js", workers_dev: true, + preview_urls: false, compatibility_date: "2025-07-08", compatibility_flags: undefined, placement: undefined, @@ -104,20 +107,7 @@ describe("getRemoteConfigsDiff", () => { + \\"id\\": \\"my-kv-456\\" } ], - - \\"workers_dev\\": true, - \\"observability\\": { - \\"enabled\\": false, - \\"head_sampling_rate\\": 1, - - ... - - \\"head_sampling_rate\\": 1, - \\"invocation_logs\\": true - } - - } - + }, - + \\"workers_dev\\": true - }" + \\"workers_dev\\": true," `); expect(nonDestructive).toBe(true); }); @@ -128,6 +118,7 @@ describe("getRemoteConfigsDiff", () => { name: "silent-firefly-dbe3", main: "/tmp/src/index.js", workers_dev: true, + preview_urls: false, compatibility_date: "2025-07-08", compatibility_flags: undefined, placement: undefined, @@ -161,11 +152,11 @@ describe("getRemoteConfigsDiff", () => { ... - } }, \\"account_id\\": \\"account-id-123\\", - + \\"workers_dev\\": true - - \\"workers_dev\\": true, + \\"workers_dev\\": true, + + \\"preview_urls\\": false + - \\"preview_urls\\": false, - \\"kv_namespaces\\": [ - { - \\"binding\\": \\"MY_KV\\", @@ -191,6 +182,7 @@ describe("getRemoteConfigsDiff", () => { { observability: remoteObservability, workers_dev: true, + preview_urls: false, }, { observability: localObservability, diff --git a/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts b/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts index ba77de712a0a..f9ef5d266f21 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts @@ -1,7 +1,11 @@ import { http, HttpResponse } from "msw"; +import { ParseError } from "../../parse"; +import { getSubdomainValues } from "../../triggers/deploy"; import { mockGetWorkerSubdomain } from "./mock-workers-subdomain"; import { createFetchResult, msw } from "./msw"; import { serialize, toString } from "./serialize-form-data-entry"; +import { readWranglerConfig } from "./write-wrangler-config"; +import type { RawConfig, RawEnvironment } from "../../config"; import type { AssetConfigMetadata, WorkerMetadata, @@ -236,9 +240,30 @@ export function mockUploadWorkerRequest( ) ); } - // TODO make explicit by callers? + // Every upload is followed by a GET subdomain request, to check if the worker is enabled. + // TODO: make this explicit by callers? + let config: RawConfig = {}; + try { + config = readWranglerConfig(); + } catch (e) { + if (e instanceof ParseError) { + // Ignore, config is either bad or doesn't exist. + } else { + throw e; + } + } + let envConfig: RawEnvironment = config; + if (env) { + envConfig = config.env?.[env] ?? {}; + } + const { workers_dev, preview_urls } = getSubdomainValues( + envConfig.workers_dev, + envConfig.preview_urls, + envConfig.routes ?? [] + ); mockGetWorkerSubdomain({ - enabled: true, + enabled: workers_dev, + previews_enabled: preview_urls, env, legacyEnv, expectedScriptName, diff --git a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts index df2895697102..9e99eac7ba4f 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-workers-subdomain.ts @@ -36,7 +36,7 @@ export function mockSubDomainRequest( /** Create a mock handler to fetch the