Skip to content

Commit

Permalink
feat: support wrangler 1.x module specifiers with a deprecation warni…
Browse files Browse the repository at this point in the history
…ng (#596)
  • Loading branch information
threepointone authored Mar 15, 2022
1 parent 4341746 commit 187264d
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 5 deletions.
39 changes: 39 additions & 0 deletions .changeset/dull-fishes-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
"wrangler": patch
---

feat: support wrangler 1.x module specifiers with a deprecation warning

This implements wrangler 1.x style module specifiers, but also logs a deprecation warning for every usage.

Consider a project like so:

```
project
├── index.js
└── some-dependency.js
```

where the content of `index.js` is:

```jsx
import SomeDependency from "some-dependency.js";
addEventListener("fetch", (event) => {
// ...
});
```

`wrangler` 1.x would resolve `import SomeDependency from "some-dependency.js";` to the file `some-dependency.js`. This will work in `wrangler` v2, but it will log a deprecation warning. Instead, you should rewrite the import to specify that it's a relative path, like so:

```diff
- import SomeDependency from "some-dependency.js";
+ import SomeDependency from "./some-dependency.js";
```

In a near future version, this will become a breaking deprecation and throw an error.

(This also updates `workers-chat-demo` to use the older style specifier, since that's how it currently is at https://github.com/cloudflare/workers-chat-demo)

Known issue: This might not work as expected with `.js`/`.cjs`/`.mjs` files as expected, but that's something to be fixed overall with the module system.

Closes https://github.com/cloudflare/wrangler2/issues/586
2 changes: 1 addition & 1 deletion packages/workers-chat-demo/src/chat.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
// serve our app's static asset without relying on any separate storage. (However, the space
// available for assets served this way is very limited; larger sites should continue to use Workers
// KV to serve assets.)
import HTML from "./chat.html";
import HTML from "chat.html";

// `handleErrors()` is a little utility function that can wrap an HTTP request handler in a
// try/catch and return errors to the client. You probably wouldn't want to use this in production
Expand Down
57 changes: 56 additions & 1 deletion packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1966,6 +1966,62 @@ export default{
`);
});
});

describe("legacy module specifiers", () => {
it("should work with legacy module specifiers, with a deprecation warning (1)", async () => {
writeWranglerToml({
rules: [{ type: "Text", globs: ["**/*.file"], fallthrough: false }],
});
fs.writeFileSync(
"./index.js",
`import TEXT from 'text.file'; export default {};`
);
fs.writeFileSync("./text.file", "SOME TEXT CONTENT");
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedModules: {
"./2d91d1c4dd6e57d4f5432187ab7c25f45a8973f0-text.file":
"SOME TEXT CONTENT",
},
});
await runWrangler("publish index.js");
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(
`"Deprecation warning: detected a legacy module import in \\"./index.js\\". This will stop working in the future. Replace references to \\"text.file\\" with \\"./text.file\\";"`
);
});
});

it("should work with legacy module specifiers, with a deprecation warning (2)", async () => {
writeWranglerToml();
fs.writeFileSync(
"./index.js",
`import WASM from 'index.wasm'; export default {};`
);
fs.writeFileSync("./index.wasm", "SOME WASM CONTENT");
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedModules: {
"./94b240d0d692281e6467aa42043986e5c7eea034-index.wasm":
"SOME WASM CONTENT",
},
});
await runWrangler("publish index.js");
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(
`"Deprecation warning: detected a legacy module import in \\"./index.js\\". This will stop working in the future. Replace references to \\"index.wasm\\" with \\"./index.wasm\\";"`
);
});
});

/** Write a mock Worker script to disk. */
Expand Down Expand Up @@ -2044,7 +2100,6 @@ function mockUploadWorkerRequest({
expectedEntry
);
}

const metadata = JSON.parse(
formBody.get("metadata") as string
) as WorkerMetadata;
Expand Down
20 changes: 18 additions & 2 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import assert from "node:assert";
import * as fs from "node:fs";
import * as path from "node:path";
import * as esbuild from "esbuild";
import makeModuleCollector from "./module-collection";
import createModuleCollector from "./module-collection";
import type { Config } from "./config";
import type { Entry } from "./entry";
import type { CfModule } from "./worker";
Expand Down Expand Up @@ -30,7 +30,23 @@ export async function bundleWorker(
): Promise<BundleResult> {
const { serveAssetsFromWorker, jsxFactory, jsxFragment, rules, watch } =
options;
const moduleCollector = makeModuleCollector({ format: entry.format, rules });
const entryDirectory = path.dirname(entry.file);
const moduleCollector = createModuleCollector({
wrangler1xlegacyModuleReferences: {
rootDirectory: entryDirectory,
fileNames: new Set(
fs
.readdirSync(entryDirectory, { withFileTypes: true })
.filter(
(dirEntry) =>
dirEntry.isFile() && dirEntry.name !== path.basename(entry.file)
)
.map((dirEnt) => dirEnt.name)
),
},
format: entry.format,
rules,
});
const result = await esbuild.build({
...getEntryPoint(entry.file, serveAssetsFromWorker),
bundle: true,
Expand Down
79 changes: 78 additions & 1 deletion packages/wrangler/src/module-collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,15 @@ export const DEFAULT_MODULE_RULES: Config["rules"] = [
{ type: "CompiledWasm", globs: ["**/*.wasm"] },
];

export default function makeModuleCollector(props: {
export default function createModuleCollector(props: {
format: CfScriptFormat;
rules?: Config["rules"];
// a collection of "legacy" style module references, which are just file names
// we will eventually deprecate this functionality, hence the verbose greppable name
wrangler1xlegacyModuleReferences: {
rootDirectory: string;
fileNames: Set<string>;
};
}): {
modules: CfModule[];
plugin: esbuild.Plugin;
Expand All @@ -41,6 +47,10 @@ export default function makeModuleCollector(props: {
...DEFAULT_MODULE_RULES!,
];

// First, we want to add some validations to the module rules
// We want to warn if rules are accidentally configured in such a way that
// subsequent rules will never match because `fallthrough` hasn't been set

const completedRuleLocations: Record<string, number> = {};
let index = 0;
const rulesToRemove: Config["rules"] = [];
Expand Down Expand Up @@ -92,6 +102,73 @@ export default function makeModuleCollector(props: {
modules.splice(0);
});

// ~ start legacy module specifier support ~

// This section detects usage of "legacy" 1.x style module specifiers
// and modifies them so they "work" in wrangler v2, but with a warning

const rulesMatchers = rules.flatMap((rule) => {
return rule.globs.map((glob) => {
const regex = globToRegExp(glob);
return {
regex,
rule,
};
});
});

build.onResolve(
{
filter: new RegExp(
[...props.wrangler1xlegacyModuleReferences.fileNames]
.map((fileName) => `^${fileName}$`)
.join("|")
),
},
async (args: esbuild.OnResolveArgs) => {
// In the future, this will simply throw an error
console.warn(
`Deprecation warning: detected a legacy module import in "./${path.relative(
process.cwd(),
args.importer
)}". This will stop working in the future. Replace references to "${
args.path
}" with "./${args.path}";`
);

// take the file and massage it to a
// transportable/manageable format
const filePath = path.join(
props.wrangler1xlegacyModuleReferences.rootDirectory,
args.path
);
const fileContent = await readFile(filePath);
const fileHash = crypto
.createHash("sha1")
.update(fileContent)
.digest("hex");
const fileName = `./${fileHash}-${path.basename(args.path)}`;

const { rule } =
rulesMatchers.find(({ regex }) => regex.test(fileName)) || {};
if (rule) {
// add the module to the array
modules.push({
name: fileName,
content: fileContent,
type: RuleTypeToModuleType[rule.type],
});
return {
path: fileName, // change the reference to the changed module
external: props.format === "modules", // mark it as external in the bundle
namespace: `wrangler-module-${rule.type}`, // just a tag, this isn't strictly necessary
watchFiles: [filePath], // we also add the file to esbuild's watch list
};
}
}
);
// ~ end legacy module specifier support ~

rules?.forEach((rule) => {
if (rule.type === "ESModule" || rule.type === "CommonJS") return; // TODO: we should treat these as js files, and use the jsx loader

Expand Down

0 comments on commit 187264d

Please sign in to comment.