Skip to content

Commit 8c6c672

Browse files
emily-shenemily-shen
committed
PR feedback
1 parent 809b907 commit 8c6c672

File tree

3 files changed

+135
-27
lines changed

3 files changed

+135
-27
lines changed

‎.changeset/deep-onions-move.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ Interactively handle `wrangler deploy`s that are probably assets-only, where the
66

77
For example:
88

9-
`npx wrangler deploy ./public` will now ask if you meant to deploy an folder of assets only, ask for a name, set the compat date and then ask to write your choices out to `wrangler.json` for subsequent deployments.
9+
`npx wrangler deploy ./public` will now ask if you meant to deploy a folder of assets only, ask for a name, set the compat date and then ask whether to write your choices out to `wrangler.json` for subsequent deployments.
1010

11-
`npx wrangler deploy --assets=./public` will now ask for a name, set the compat date and then ask to write your choices out to `wrangler.json` for subsequent deployments.
11+
`npx wrangler deploy --assets=./public` will now ask for a name, set the compat date and then ask whether to write your choices out to `wrangler.json` for subsequent deployments.
1212

13-
In non-interactive contexts, we will error as we currently do.
13+
In non-interactive contexts, Wrangler will error as it currently does.

‎packages/wrangler/src/__tests__/deploy.test.ts

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2729,6 +2729,7 @@ addEventListener('fetch', event => {});`
27292729
// Mock the date to ensure consistent compatibility_date
27302730
vi.setSystemTime(new Date("2024-01-01T00:00:00Z"));
27312731

2732+
// so that we can test that the name prompt defaults to the directory name
27322733
fs.mkdirSync("my-site");
27332734
process.chdir("my-site");
27342735
const assets = [
@@ -2747,6 +2748,7 @@ addEventListener('fetch', event => {});`
27472748
});
27482749
afterEach(() => {
27492750
setIsTTY(false);
2751+
vi.useRealTimers();
27502752
});
27512753

27522754
it("should handle `wrangler deploy <directory>`", async () => {
@@ -2777,7 +2779,7 @@ addEventListener('fetch', event => {});`
27772779
},
27782780
},
27792781
});
2780-
expect(fs.readFileSync("wrangler.json", "utf-8"))
2782+
expect(fs.readFileSync("wrangler.jsonc", "utf-8"))
27812783
.toMatchInlineSnapshot(`
27822784
"{
27832785
\\"name\\": \\"test-name\\",
@@ -2801,8 +2803,10 @@ addEventListener('fetch', event => {});`
28012803
\\"directory\\": \\"./assets\\"
28022804
}
28032805
}
2804-
to <cwd>/wrangler.json.
2805-
Please run wrangler deploy instead of wrangler deploy ./assets next time. Wrangler will automatically use the configuration saved to wrangler.json.
2806+
to <cwd>/wrangler.jsonc.
2807+
Please run \`wrangler deploy\` instead of \`wrangler deploy ./assets\` next time. Wrangler will automatically use the configuration saved to wrangler.jsonc.
2808+
2809+
Proceeding with deployment...
28062810
28072811
Total Upload: xx KiB / gzip: xx KiB
28082812
Worker Startup Time: 100 ms
@@ -2812,6 +2816,7 @@ addEventListener('fetch', event => {});`
28122816
Current Version ID: Galaxy-Class"
28132817
`);
28142818
});
2819+
28152820
it("should handle `wrangler deploy --assets` without name or compat date", async () => {
28162821
// if the user has used --assets flag and args.script is not set, we just need to prompt for the name and add compat date
28172822
mockPrompt({
@@ -2837,7 +2842,7 @@ addEventListener('fetch', event => {});`
28372842
},
28382843
},
28392844
});
2840-
expect(fs.readFileSync("wrangler.json", "utf-8"))
2845+
expect(fs.readFileSync("wrangler.jsonc", "utf-8"))
28412846
.toMatchInlineSnapshot(`
28422847
"{
28432848
\\"name\\": \\"test-name\\",
@@ -2860,8 +2865,112 @@ addEventListener('fetch', event => {});`
28602865
\\"directory\\": \\"./assets\\"
28612866
}
28622867
}
2863-
to <cwd>/wrangler.json.
2864-
Please run wrangler deploy instead of wrangler deploy ./assets next time. Wrangler will automatically use the configuration saved to wrangler.json.
2868+
to <cwd>/wrangler.jsonc.
2869+
Please run \`wrangler deploy\` instead of \`wrangler deploy ./assets\` next time. Wrangler will automatically use the configuration saved to wrangler.jsonc.
2870+
2871+
Proceeding with deployment...
2872+
2873+
Total Upload: xx KiB / gzip: xx KiB
2874+
Worker Startup Time: 100 ms
2875+
Uploaded test-name (TIMINGS)
2876+
Deployed test-name triggers (TIMINGS)
2877+
https://test-name.test-sub-domain.workers.dev
2878+
Current Version ID: Galaxy-Class"
2879+
`);
2880+
});
2881+
2882+
it("should suggest 'my-project' if the default name from the cwd is invalid", async () => {
2883+
process.chdir("../");
2884+
fs.renameSync("my-site", "[blah]");
2885+
process.chdir("[blah]");
2886+
// if the user has used --assets flag and args.script is not set, we just need to prompt for the name and add compat date
2887+
mockPrompt({
2888+
text: "What do you want to name your project?",
2889+
// not [blah] because it is an invalid worker name
2890+
options: { defaultValue: "my-project" },
2891+
result: "test-name",
2892+
});
2893+
mockConfirm({
2894+
text: "Do you want Wrangler to write a wrangler.json config file to store this configuration?\nThis will allow you to simply run `wrangler deploy` on future deployments.",
2895+
result: true,
2896+
});
2897+
2898+
const bodies: AssetManifest[] = [];
2899+
await mockAUSRequest(bodies);
2900+
2901+
await runWrangler("deploy --assets ./assets");
2902+
expect(bodies.length).toBe(1);
2903+
expect(bodies[0]).toEqual({
2904+
manifest: {
2905+
"/index.html": {
2906+
hash: "8308ce789f3d08668ce87176838d59d0",
2907+
size: 17,
2908+
},
2909+
},
2910+
});
2911+
expect(fs.readFileSync("wrangler.jsonc", "utf-8"))
2912+
.toMatchInlineSnapshot(`
2913+
"{
2914+
\\"name\\": \\"test-name\\",
2915+
\\"compatibility_date\\": \\"2024-01-01\\",
2916+
\\"assets\\": {
2917+
\\"directory\\": \\"./assets\\"
2918+
}
2919+
}"
2920+
`);
2921+
});
2922+
2923+
it("should bail if the user denies that they are trying to deploy a directory", async () => {
2924+
mockConfirm({
2925+
text: "It looks like you are trying to deploy a directory of static assets only. Is this correct?",
2926+
result: false,
2927+
});
2928+
2929+
await expect(runWrangler("deploy ./assets")).rejects
2930+
.toThrowErrorMatchingInlineSnapshot(`
2931+
[Error: The entry-point file at "assets" was not found.
2932+
The provided entry-point path, "assets", points to a directory, rather than a file.
2933+
2934+
If you want to deploy a directory of static assets, you can do so by using the \`--assets\` flag. For example:
2935+
2936+
wrangler deploy --assets=./assets
2937+
]
2938+
`);
2939+
});
2940+
2941+
it("does not write out a wrangler config file if the user says no", async () => {
2942+
mockPrompt({
2943+
text: "What do you want to name your project?",
2944+
options: { defaultValue: "my-site" },
2945+
result: "test-name",
2946+
});
2947+
mockConfirm({
2948+
text: "Do you want Wrangler to write a wrangler.json config file to store this configuration?\nThis will allow you to simply run `wrangler deploy` on future deployments.",
2949+
result: false,
2950+
});
2951+
2952+
const bodies: AssetManifest[] = [];
2953+
await mockAUSRequest(bodies);
2954+
2955+
await runWrangler("deploy --assets ./assets");
2956+
expect(bodies.length).toBe(1);
2957+
expect(bodies[0]).toEqual({
2958+
manifest: {
2959+
"/index.html": {
2960+
hash: "8308ce789f3d08668ce87176838d59d0",
2961+
size: 17,
2962+
},
2963+
},
2964+
});
2965+
expect(fs.existsSync("wrangler.jsonc")).toBe(false);
2966+
expect(std.out).toMatchInlineSnapshot(`
2967+
"
2968+
2969+
No compatibility date found Defaulting to today: 2024-01-01
2970+
2971+
You should run wrangler deploy --name test-name --compatibility-date 2024-01-01 --assets ./assets next time to deploy this Worker without going through this flow again.
2972+
2973+
Proceeding with deployment...
28652974
28662975
Total Upload: xx KiB / gzip: xx KiB
28672976
Worker Startup Time: 100 ms

‎packages/wrangler/src/deploy/index.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ export const deployCommand = createCommand({
256256
try {
257257
const stats = statSync(args.script);
258258
if (stats.isDirectory()) {
259-
args = await handleMaybeAssetsDeployment(args);
259+
args = await handleMaybeAssetsDeployment(args.script, args);
260260
}
261261
} catch (error) {
262262
// If this is our UserError, re-throw it
@@ -268,7 +268,7 @@ export const deployCommand = createCommand({
268268
}
269269
// atttempt to interactively handle `wrangler deploy --assets <directory>` missing compat date or name
270270
else if (args.assets && (!args.compatibilityDate || !args.name)) {
271-
args = await handleMaybeAssetsDeployment(args);
271+
args = await handleMaybeAssetsDeployment(args.assets, args);
272272
}
273273
}
274274

@@ -391,12 +391,15 @@ export const deployCommand = createCommand({
391391
export type DeployArgs = (typeof deployCommand)["args"];
392392

393393
/**
394-
* Handles the case where a user provides a directory as a positional argument,
395-
* probably intending to deploy static assets. e.g. wrangler deploy ./public
396-
* We then interactively take the user through deployment (missing name, compatibility date, etc.)
397-
* and try and output this as a wrangler.json for future deployments.
394+
* Handles the case where:
395+
* - a user provides a directory as a positional argument probably intending to deploy static assets. e.g. wrangler deploy ./public
396+
* - a user provides `--assets` but does not provide a name or compatibility date.
397+
* We then interactively take the user through deployment (missing name and/or compatibility date)
398+
* and ask to output this as a wrangler.jsonc for future deployments.
399+
* If this successfully completes, continue deploying with the updated values.
398400
*/
399401
export async function handleMaybeAssetsDeployment(
402+
assetDirectory: string,
400403
args: DeployArgs
401404
): Promise<DeployArgs> {
402405
if (isNonInteractiveOrCI()) {
@@ -412,7 +415,7 @@ export async function handleMaybeAssetsDeployment(
412415
);
413416
logger.log("");
414417
if (deployAssets) {
415-
args.assets = args.script;
418+
args.assets = assetDirectory;
416419
args.script = undefined;
417420
} else {
418421
// let the usual error handling path kick in
@@ -422,21 +425,18 @@ export async function handleMaybeAssetsDeployment(
422425

423426
// Check if name is provided, if not ask for it
424427
if (!args.name) {
428+
const defaultName = process.cwd().split(path.sep).pop()?.replace("_", "-");
429+
const isValidName = defaultName && /^[a-zA-Z0-9-]+$/.test(defaultName);
425430
const projectName = await prompt("What do you want to name your project?", {
426-
defaultValue: process.cwd().split(path.sep).pop(),
431+
defaultValue: isValidName ? defaultName : "my-project",
427432
});
428433
args.name = projectName;
429434
logger.log("");
430435
}
431436

432437
// Set compatibility date if not provided
433438
if (!args.compatibilityDate) {
434-
const today = new Date();
435-
const compatibilityDate = [
436-
today.getFullYear(),
437-
(today.getMonth() + 1).toString().padStart(2, "0"),
438-
today.getDate().toString().padStart(2, "0"),
439-
].join("-");
439+
const compatibilityDate = formatCompatibilityDate(new Date());
440440
args.compatibilityDate = compatibilityDate;
441441
logger.log(
442442
`${chalk.bold("No compatibility date found")} Defaulting to today:`,
@@ -451,7 +451,7 @@ export async function handleMaybeAssetsDeployment(
451451
);
452452

453453
if (writeConfig) {
454-
const configPath = path.join(process.cwd(), "wrangler.json");
454+
const configPath = path.join(process.cwd(), "wrangler.jsonc");
455455
const jsonString = JSON.stringify(
456456
{
457457
name: args.name,
@@ -464,16 +464,15 @@ export async function handleMaybeAssetsDeployment(
464464
writeFileSync(configPath, jsonString);
465465
logger.log(`Wrote \n${jsonString}\n to ${chalk.bold(configPath)}.`);
466466
logger.log(
467-
`Please run ${chalk.bold("wrangler deploy")} instead of ${chalk.bold(`wrangler deploy ${args.assets}`)} next time. Wrangler will automatically use the configuration saved to wrangler.json.`
467+
`Please run ${chalk.bold("`wrangler deploy`")} instead of ${chalk.bold(`\`wrangler deploy ${args.assets}\``)} next time. Wrangler will automatically use the configuration saved to wrangler.jsonc.`
468468
);
469469
} else {
470-
logger.log("Proceeding with deployment...");
471470
logger.log(
472471
`You should run ${chalk.bold(
473472
`wrangler deploy --name ${args.name} --compatibility-date ${args.compatibilityDate} --assets ${args.assets}`
474473
)} next time to deploy this Worker without going through this flow again.`
475474
);
476475
}
477-
logger.log("");
476+
logger.log("\nProceeding with deployment...\n");
478477
return args;
479478
}

0 commit comments

Comments
 (0)