Skip to content

Commit 551fd9a

Browse files
petebacondarwinpetebacondarwin
committed
refactor to simplify findAdditionalModules()
1 parent 7e7a95a commit 551fd9a

File tree

11 files changed

+163
-185
lines changed

11 files changed

+163
-185
lines changed

‎packages/wrangler/src/__tests__/traverse-module-graph.test.ts‎ renamed to ‎packages/wrangler/src/__tests__/find-additional-modules.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { mkdir, writeFile } from "fs/promises";
22
import path from "path";
33
import dedent from "ts-dedent";
4-
import traverseModuleGraph from "../deployment-bundle/traverse-module-graph";
4+
import findAdditionalModules from "../deployment-bundle/find-additional-modules";
55
import { mockConsoleMethods } from "./helpers/mock-console";
66
import { runInTempDir } from "./helpers/run-in-tmp";
77
import type { ConfigModuleRuleType } from "../config";
@@ -36,7 +36,7 @@ describe("traverse module graph", () => {
3636
`
3737
);
3838

39-
const bundle = await traverseModuleGraph(
39+
const modules = await findAdditionalModules(
4040
{
4141
file: path.join(process.cwd(), "./index.js"),
4242
directory: process.cwd(),
@@ -46,7 +46,7 @@ describe("traverse module graph", () => {
4646
[]
4747
);
4848

49-
expect(bundle.modules).toStrictEqual([]);
49+
expect(modules).toStrictEqual([]);
5050
});
5151

5252
it.each([
@@ -71,7 +71,7 @@ describe("traverse module graph", () => {
7171
`
7272
);
7373

74-
const bundle = await traverseModuleGraph(
74+
const modules = await findAdditionalModules(
7575
{
7676
file: path.join(process.cwd(), "./index.js"),
7777
directory: process.cwd(),
@@ -81,7 +81,7 @@ describe("traverse module graph", () => {
8181
[{ type: type as ConfigModuleRuleType, globs: ["**/*.js"] }]
8282
);
8383

84-
expect(bundle.modules[0].type).toStrictEqual(format);
84+
expect(modules[0].type).toStrictEqual(format);
8585
});
8686

8787
it("should not resolve JS outside the module root", async () => {
@@ -104,7 +104,7 @@ describe("traverse module graph", () => {
104104
`
105105
);
106106

107-
const bundle = await traverseModuleGraph(
107+
const modules = await findAdditionalModules(
108108
{
109109
file: path.join(process.cwd(), "./src/nested/index.js"),
110110
directory: path.join(process.cwd(), "./src/nested"),
@@ -115,7 +115,7 @@ describe("traverse module graph", () => {
115115
[{ type: "ESModule", globs: ["**/*.js"] }]
116116
);
117117

118-
expect(bundle.modules).toStrictEqual([]);
118+
expect(modules).toStrictEqual([]);
119119
});
120120

121121
it("should resolve JS with module root", async () => {
@@ -138,7 +138,7 @@ describe("traverse module graph", () => {
138138
`
139139
);
140140

141-
const bundle = await traverseModuleGraph(
141+
const modules = await findAdditionalModules(
142142
{
143143
file: path.join(process.cwd(), "./src/nested/index.js"),
144144
directory: path.join(process.cwd(), "./src/nested"),
@@ -149,7 +149,7 @@ describe("traverse module graph", () => {
149149
[{ type: "ESModule", globs: ["**/*.js"] }]
150150
);
151151

152-
expect(bundle.modules[0].name).toStrictEqual("other.js");
152+
expect(modules[0].name).toStrictEqual("other.js");
153153
});
154154

155155
it("should ignore files not matched by glob", async () => {
@@ -172,7 +172,7 @@ describe("traverse module graph", () => {
172172
`
173173
);
174174

175-
const bundle = await traverseModuleGraph(
175+
const modules = await findAdditionalModules(
176176
{
177177
file: path.join(process.cwd(), "./src/nested/index.js"),
178178
directory: path.join(process.cwd(), "./src/nested"),
@@ -183,7 +183,7 @@ describe("traverse module graph", () => {
183183
[{ type: "ESModule", globs: ["**/*.mjs"] }]
184184
);
185185

186-
expect(bundle.modules.length).toStrictEqual(0);
186+
expect(modules.length).toStrictEqual(0);
187187
});
188188

189189
it("should resolve files that match the default rules", async () => {
@@ -206,7 +206,7 @@ describe("traverse module graph", () => {
206206
`
207207
);
208208

209-
const bundle = await traverseModuleGraph(
209+
const modules = await findAdditionalModules(
210210
{
211211
file: path.join(process.cwd(), "./src/index.js"),
212212
directory: path.join(process.cwd(), "./src"),
@@ -217,6 +217,6 @@ describe("traverse module graph", () => {
217217
[]
218218
);
219219

220-
expect(bundle.modules[0].name).toStrictEqual("other.txt");
220+
expect(modules[0].name).toStrictEqual("other.txt");
221221
});
222222
});

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
printBundleSize,
1212
printOffendingDependencies,
1313
} from "../deployment-bundle/bundle-reporter";
14+
import { getBundleType } from "../deployment-bundle/bundle-type";
1415
import { createWorkerUploadForm } from "../deployment-bundle/create-worker-upload-form";
1516
import findAdditionalModules from "../deployment-bundle/find-additional-modules";
1617
import { addHyphens } from "../deployments";
@@ -454,7 +455,12 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
454455

455456
const { modules, dependencies, resolvedEntryPointPath, bundleType } =
456457
props.noBundle
457-
? await findAdditionalModules(props.entry, props.rules)
458+
? {
459+
modules: await findAdditionalModules(props.entry, props.rules),
460+
dependencies: {},
461+
resolvedEntryPointPath: props.entry.file,
462+
bundleType: getBundleType(props.entry.format),
463+
}
458464
: await bundleWorker(
459465
props.entry,
460466
typeof destination === "string" ? destination : destination.path,

‎packages/wrangler/src/deployment-bundle/bundle-type.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import type { CfModuleType } from "./worker";
2+
3+
/**
4+
* Compute the entry-point type from the bundle format.
5+
*/
6+
export function getBundleType(format: string): CfModuleType {
7+
return format === "modules" ? "esm" : "commonjs";
8+
}

‎packages/wrangler/src/deployment-bundle/bundle.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
44
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
55
import * as esbuild from "esbuild";
66
import tmp from "tmp-promise";
7-
import createModuleCollector from "../module-collection";
87
import { getBasePath } from "../paths";
98
import { applyMiddlewareLoaderFacade } from "./apply-middleware";
109
import {
@@ -16,12 +15,13 @@ import { getEntryPointFromMetafile } from "./entry-point-from-metafile";
1615
import { cloudflareInternalPlugin } from "./esbuild-plugins/cloudflare-internal";
1716
import { configProviderPlugin } from "./esbuild-plugins/config-provider";
1817
import { nodejsCompatPlugin } from "./esbuild-plugins/nodejs-compat";
18+
import createModuleCollector from "./module-collection";
1919
import type { Config } from "../config";
2020
import type { DurableObjectBindings } from "../config/environment";
2121
import type { WorkerRegistry } from "../dev-registry";
22-
import type { ModuleCollector } from "../module-collection";
2322
import type { MiddlewareLoader } from "./apply-middleware";
2423
import type { Entry } from "./entry";
24+
import type { ModuleCollector } from "./module-collection";
2525
import type { CfModule } from "./worker";
2626

2727
export const COMMON_ESBUILD_OPTIONS = {

‎packages/wrangler/src/deployment-bundle/find-additional-modules.ts‎

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import { readdir } from "node:fs/promises";
1+
import { readdir, readFile } from "node:fs/promises";
22
import path from "node:path";
33
import chalk from "chalk";
4+
import globToRegExp from "glob-to-regexp";
45
import { logger } from "../logger";
5-
import { matchFiles, parseRules } from "../module-collection";
6-
import type { Config } from "../config";
7-
import type { BundleResult } from "./bundle";
6+
import { parseRules, RuleTypeToModuleType } from "./module-collection";
7+
import type { Rule } from "../config/environment";
88
import type { Entry } from "./entry";
9+
import type { CfModule } from "./worker";
910

1011
async function getFiles(root: string, relativeTo: string): Promise<string[]> {
1112
const files = [];
@@ -31,8 +32,8 @@ async function getFiles(root: string, relativeTo: string): Promise<string[]> {
3132
*/
3233
export default async function findAdditionalModules(
3334
entry: Entry,
34-
rules: Config["rules"]
35-
): Promise<BundleResult> {
35+
rules: Rule[]
36+
): Promise<CfModule[]> {
3637
const files = await getFiles(entry.moduleRoot, entry.moduleRoot);
3738
const relativeEntryPoint = path
3839
.relative(entry.moduleRoot, entry.file)
@@ -45,23 +46,73 @@ export default async function findAdditionalModules(
4546
name: m.name,
4647
}));
4748

48-
const bundleType = entry.format === "modules" ? "esm" : "commonjs";
49-
5049
if (modules.length > 0) {
5150
logger.info(`Attaching additional modules:`);
5251
modules.forEach(({ name, type }) => {
5352
logger.info(`- ${chalk.blue(name)} (${chalk.green(type ?? "")})`);
5453
});
5554
}
5655

57-
return {
58-
modules,
59-
dependencies: {},
60-
resolvedEntryPointPath: entry.file,
61-
bundleType,
62-
stop: undefined,
63-
sourceMapPath: undefined,
64-
sourceMapMetadata: undefined,
65-
moduleCollector: undefined,
66-
};
56+
return modules;
57+
}
58+
59+
async function matchFiles(
60+
files: string[],
61+
relativeTo: string,
62+
{ rules, removedRules }: { rules: Rule[]; removedRules: Rule[] }
63+
) {
64+
const modules: CfModule[] = [];
65+
66+
// Deduplicate modules. This is usually a poorly specified `wrangler.toml` configuration, but duplicate modules will cause a crash at runtime
67+
const moduleNames = new Set<string>();
68+
for (const rule of rules) {
69+
for (const glob of rule.globs) {
70+
const regexp = globToRegExp(glob, {
71+
globstar: true,
72+
});
73+
const newModules = await Promise.all(
74+
files
75+
.filter((f) => regexp.test(f))
76+
.map(async (name) => {
77+
const filePath = name;
78+
const fileContent = await readFile(path.join(relativeTo, filePath));
79+
80+
return {
81+
name: filePath,
82+
content: fileContent,
83+
type: RuleTypeToModuleType[rule.type],
84+
};
85+
})
86+
);
87+
for (const module of newModules) {
88+
if (!moduleNames.has(module.name)) {
89+
moduleNames.add(module.name);
90+
modules.push(module);
91+
} else {
92+
logger.warn(
93+
`Ignoring duplicate module: ${chalk.blue(
94+
module.name
95+
)} (${chalk.green(module.type ?? "")})`
96+
);
97+
}
98+
}
99+
}
100+
}
101+
102+
// This is just a sanity check verifying that no files match rules that were removed
103+
for (const rule of removedRules) {
104+
for (const glob of rule.globs) {
105+
const regexp = globToRegExp(glob);
106+
for (const file of files) {
107+
if (regexp.test(file)) {
108+
throw new Error(
109+
`The file ${file} matched a module rule in your configuration (${JSON.stringify(
110+
rule
111+
)}), but was ignored because a previous rule with the same type was not marked as \`fallthrough = true\`.`
112+
);
113+
}
114+
}
115+
}
116+
}
117+
return modules;
67118
}

‎packages/wrangler/src/module-collection.ts‎ renamed to ‎packages/wrangler/src/deployment-bundle/module-collection.ts‎

Lines changed: 11 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
11
import crypto from "node:crypto";
22
import { readFile } from "node:fs/promises";
33
import path from "node:path";
4-
import chalk from "chalk";
54
import globToRegExp from "glob-to-regexp";
6-
import { logger } from "./logger";
7-
import type { Config, ConfigModuleRuleType } from "./config";
8-
import type {
9-
CfModule,
10-
CfModuleType,
11-
CfScriptFormat,
12-
} from "./deployment-bundle/worker";
5+
import { logger } from "../logger";
6+
import type { Config, ConfigModuleRuleType } from "../config";
7+
import type { CfModule, CfModuleType, CfScriptFormat } from "./worker";
138
import type esbuild from "esbuild";
149

1510
function flipObject<
@@ -19,13 +14,14 @@ function flipObject<
1914
return Object.fromEntries(Object.entries(obj).map(([k, v]) => [v, k]));
2015
}
2116

22-
const RuleTypeToModuleType: Record<ConfigModuleRuleType, CfModuleType> = {
23-
ESModule: "esm",
24-
CommonJS: "commonjs",
25-
CompiledWasm: "compiled-wasm",
26-
Data: "buffer",
27-
Text: "text",
28-
};
17+
export const RuleTypeToModuleType: Record<ConfigModuleRuleType, CfModuleType> =
18+
{
19+
ESModule: "esm",
20+
CommonJS: "commonjs",
21+
CompiledWasm: "compiled-wasm",
22+
Data: "buffer",
23+
Text: "text",
24+
};
2925

3026
export const ModuleTypeToRuleType = flipObject(RuleTypeToModuleType);
3127

@@ -89,70 +85,6 @@ export function parseRules(userRules: Config["rules"] = []) {
8985
return { rules, removedRules: rulesToRemove };
9086
}
9187

92-
export async function matchFiles(
93-
files: string[],
94-
relativeTo: string,
95-
{
96-
rules,
97-
removedRules,
98-
}: { rules: Config["rules"]; removedRules: Config["rules"] }
99-
) {
100-
const modules: CfModule[] = [];
101-
102-
// Deduplicate modules. This is usually a poorly specified `wrangler.toml` configuration, but duplicate modules will cause a crash at runtime
103-
const moduleNames = new Set<string>();
104-
for (const rule of rules) {
105-
for (const glob of rule.globs) {
106-
const regexp = globToRegExp(glob, {
107-
globstar: true,
108-
});
109-
const newModules = await Promise.all(
110-
files
111-
.filter((f) => regexp.test(f))
112-
.map(async (name) => {
113-
const filePath = name;
114-
const fileContent = await readFile(path.join(relativeTo, filePath));
115-
116-
return {
117-
name: filePath,
118-
content: fileContent,
119-
type: RuleTypeToModuleType[rule.type],
120-
};
121-
})
122-
);
123-
for (const module of newModules) {
124-
if (!moduleNames.has(module.name)) {
125-
moduleNames.add(module.name);
126-
modules.push(module);
127-
} else {
128-
logger.warn(
129-
`Ignoring duplicate module: ${chalk.blue(
130-
module.name
131-
)} (${chalk.green(module.type ?? "")})`
132-
);
133-
}
134-
}
135-
}
136-
}
137-
138-
// This is just a sanity check verifying that no files match rules that were removed
139-
for (const rule of removedRules) {
140-
for (const glob of rule.globs) {
141-
const regexp = globToRegExp(glob);
142-
for (const file of files) {
143-
if (regexp.test(file)) {
144-
throw new Error(
145-
`The file ${file} matched a module rule in your configuration (${JSON.stringify(
146-
rule
147-
)}), but was ignored because a previous rule with the same type was not marked as \`fallthrough = true\`.`
148-
);
149-
}
150-
}
151-
}
152-
}
153-
return modules;
154-
}
155-
15688
export type ModuleCollector = {
15789
modules: CfModule[];
15890
plugin: esbuild.Plugin;

0 commit comments

Comments
 (0)