Skip to content

Commit

Permalink
[wrangler] Add legacy_assets config/flag and deprecation warnings for…
Browse files Browse the repository at this point in the history 
… --assets (#6272)

* add legacy assets config key and flag and replace references to assets

* add deprecation warnings for assets/legacy-assets

* fix and add tests

* Revert unnecessary change to wrangler secret.test.ts

* changeset

* unalias --assets and --legacy-assets to allow separate warnings

---------

Co-authored-by: emily-shen <eshen@cloudflare.com>
  • Loading branch information
@emily-shen
emily-shen and emily-shen committed Jul 22, 2024
1 parent 20c0bf1 commit 084d39e
Show file tree
Hide file tree
Showing 21 changed files with 613 additions and 379 deletions.
12 changes: 12 additions & 0 deletions .changeset/yellow-pumpkins-hug.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": minor
---

---

## "wrangler": minor

fix: add `legacy-assets` config and flag as alias of current `assets` behavior

- The existing behavior of the `assets` config key/flag will change on August 15th.
- `legacy-assets` will preserve current functionality.
142 changes: 95 additions & 47 deletions packages/wrangler/src/__tests__/configuration.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("normalizeAndValidateConfig()", () => {

expect(config).toEqual({
account_id: undefined,
assets: undefined,
legacy_assets: undefined,
build: {
command: undefined,
cwd: undefined,
Expand Down Expand Up @@ -445,61 +445,60 @@ describe("normalizeAndValidateConfig()", () => {
});
});

describe("[assets]", () => {
describe("[legacy_assets]", () => {
it("normalizes a string input to an object", () => {
const { config, diagnostics } = normalizeAndValidateConfig(
{
assets: "path/to/assets",
legacy_assets: "path/to/assets",
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(config.assets).toMatchInlineSnapshot(`
Object {
"browser_TTL": undefined,
"bucket": "path/to/assets",
"exclude": Array [],
"include": Array [],
"serve_single_page_app": false,
}
`);
expect(config.legacy_assets).toMatchInlineSnapshot(`
Object {
"browser_TTL": undefined,
"bucket": "path/to/assets",
"exclude": Array [],
"include": Array [],
"serve_single_page_app": false,
}
`);
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.hasErrors()).toBe(false);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- \\"assets\\" fields are experimental and may change or break at any time."
`);
"Processing wrangler configuration:
- \\"legacy_assets\\" fields are experimental and may change or break at any time."
`);
});

it("errors when input is not a string or object", () => {
const { config, diagnostics } = normalizeAndValidateConfig(
{
assets: 123,
legacy_assets: 123,
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(config.assets).toBeUndefined();
expect(config.legacy_assets).toBeUndefined();
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.hasErrors()).toBe(true);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- \\"assets\\" fields are experimental and may change or break at any time."
`);
"Processing wrangler configuration:
- \\"legacy_assets\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Expected the \`assets\` field to be a string or an object, but got number."
`);
"Processing wrangler configuration:
- Expected the \`legacy_assets\` field to be a string or an object, but got number."
`);
});

it("should error if `assets` config is missing `bucket`", () => {
it("should error if `legacy_assets` config is missing `bucket`", () => {
const expectedConfig: RawConfig = {
// @ts-expect-error we're intentionally passing an invalid configuration here
assets: {
legacy_assets: {
include: ["INCLUDE_1", "INCLUDE_2"],
exclude: ["EXCLUDE_1", "EXCLUDE_2"],
},
Expand All @@ -511,25 +510,25 @@ describe("normalizeAndValidateConfig()", () => {
{ env: undefined }
);

expect(config.assets).toEqual(
expect.objectContaining(expectedConfig.assets)
expect(config.legacy_assets).toEqual(
expect.objectContaining(expectedConfig.legacy_assets)
);
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.hasErrors()).toBe(true);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- \\"assets\\" fields are experimental and may change or break at any time."
`);
"Processing wrangler configuration:
- \\"legacy_assets\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- \\"assets.bucket\\" is a required field."
`);
"Processing wrangler configuration:
- \\"legacy_assets.bucket\\" is a required field."
`);
});

it("should error on invalid `assets` values", () => {
it("should error on invalid `legacy_assets` values", () => {
const expectedConfig = {
assets: {
legacy_assets: {
bucket: "BUCKET",
include: [222, 333],
exclude: [444, 555],
Expand All @@ -547,18 +546,67 @@ describe("normalizeAndValidateConfig()", () => {
expect(config).toEqual(expect.objectContaining(expectedConfig));
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- \\"assets\\" fields are experimental and may change or break at any time."
`);
"Processing wrangler configuration:
- \\"legacy_assets\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Expected \\"assets.include.[0]\\" to be of type string but got 222.
- Expected \\"assets.include.[1]\\" to be of type string but got 333.
- Expected \\"assets.exclude.[0]\\" to be of type string but got 444.
- Expected \\"assets.exclude.[1]\\" to be of type string but got 555.
- Expected \\"assets.browser_TTL\\" to be of type number but got \\"not valid\\".
- Expected \\"assets.serve_single_page_app\\" to be of type boolean but got \\"INVALID\\"."
`);
"Processing wrangler configuration:
- Expected \\"legacy_assets.include.[0]\\" to be of type string but got 222.
- Expected \\"legacy_assets.include.[1]\\" to be of type string but got 333.
- Expected \\"legacy_assets.exclude.[0]\\" to be of type string but got 444.
- Expected \\"legacy_assets.exclude.[1]\\" to be of type string but got 555.
- Expected \\"legacy_assets.browser_TTL\\" to be of type number but got \\"not valid\\".
- Expected \\"legacy_assets.serve_single_page_app\\" to be of type boolean but got \\"INVALID\\"."
`);
});

it("saves `assets` values under `legacy_assets`", () => {
const { config, diagnostics } = normalizeAndValidateConfig(
{
assets: "path/to/assets",
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(config.legacy_assets).toMatchInlineSnapshot(`
Object {
"browser_TTL": undefined,
"bucket": "path/to/assets",
"exclude": Array [],
"include": Array [],
"serve_single_page_app": false,
}
`);
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.hasErrors()).toBe(false);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Behavior change: \\"assets\\":
The \`assets\` feature is experimental. We are going to be changing its behavior on August 15th.
Releases of wrangler after this date will no longer support current functionality.
Please shift to \`legacy_assets\` to preserve the current functionality. "
`);
});

it("should error if `assets` and `legacy_assets` are both defined", () => {
const { diagnostics } = normalizeAndValidateConfig(
{
assets: "path/to/assets",
legacy_assets: "path/to/assets",
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.hasErrors()).toBe(true);

expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Expected only one of \`assets\` or \`legacy_assets\`."
`);
});
});

Expand Down
Loading

0 comments on commit 084d39e

Please sign in to comment.