Skip to content

Commit

Permalink
feat: support wrangler 1.x module specifiers with a deprecation warning
Browse files Browse the repository at this point in the history
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)

Closes #586
  • Loading branch information
threepointone committed Mar 14, 2022
1 parent 1cb7ae2 commit 1c872cd
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 7 deletions.
37 changes: 37 additions & 0 deletions .changeset/dull-fishes-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
"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)

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
30 changes: 30 additions & 0 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1966,6 +1966,36 @@ export default{
`);
});
});

describe("legacy module specifiers", () => {
it("should work with legacy module specifiers, with a deprecation warning", 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\\";"`
);
});
});
});

/** Write a mock Worker script to disk. */
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
46 changes: 42 additions & 4 deletions 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
wrangler1xlegacyModuleReferences: {
rootDirectory: string;
fileNames: Set<string>;
};
}): {
modules: CfModule[];
plugin: esbuild.Plugin;
Expand Down Expand Up @@ -97,12 +103,44 @@ export default function makeModuleCollector(props: {

rule.globs.forEach((glob) => {
build.onResolve(
{ filter: globToRegExp(glob) },
{
// we create a regexp that can detect any of the module rule
// globs, as well as any of the legacy module references
filter: new RegExp(
globToRegExp(glob).source +
"|" +
new RegExp(
[...props.wrangler1xlegacyModuleReferences.fileNames]
.map((fileName) => `^${fileName}$`)
.join("|")
).source
),
},
async (args: esbuild.OnResolveArgs) => {
const isLegacyModuleReference =
props.wrangler1xlegacyModuleReferences.fileNames.has(
args.path
);

if (isLegacyModuleReference) {
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(args.resolveDir, args.path);
const filePath = isLegacyModuleReference
? path.join(
props.wrangler1xlegacyModuleReferences.rootDirectory,
args.path
)
: path.join(args.resolveDir, args.path);
const fileContent = await readFile(filePath);
const fileHash = crypto
.createHash("sha1")
Expand Down

0 comments on commit 1c872cd

Please sign in to comment.