Skip to content

Commit 1176013

Browse files
authored
ref: Avoid direct usage of glob, extract into globFiles helper (#883)
The idea is that we can test against globFiles and eventually replace glob.
1 parent 79c1657 commit 1176013

File tree

6 files changed

+195
-34
lines changed

6 files changed

+195
-34
lines changed

packages/bundler-plugin-core/src/build-plugin-manager.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ import {
2626
serializeIgnoreOptions,
2727
stripQueryAndHashFromPath,
2828
} from "./utils";
29-
import { glob } from "glob";
3029
import { defaultRewriteSourcesHook, prepareBundleForDebugIdUpload } from "./debug-id-upload";
30+
import { globFiles } from "./glob";
3131
import { LIB_VERSION } from "./version";
3232

3333
// Module-level guard to prevent duplicate deploy records when multiple bundler plugin
@@ -678,12 +678,7 @@ export function createSentryBuildPluginManager(
678678

679679
const globResult = await startSpan(
680680
{ name: "glob", scope: sentryScope },
681-
async () =>
682-
await glob(globAssets, {
683-
absolute: true,
684-
nodir: true, // We need individual files for preparation
685-
ignore: options.sourcemaps?.ignore,
686-
})
681+
async () => await globFiles(globAssets, { ignore: options.sourcemaps?.ignore })
687682
);
688683

689684
const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => {
@@ -810,10 +805,7 @@ export function createSentryBuildPluginManager(
810805
try {
811806
const filesToDelete = await options.sourcemaps?.filesToDeleteAfterUpload;
812807
if (filesToDelete !== undefined) {
813-
const filePathsToDelete = await glob(filesToDelete, {
814-
absolute: true,
815-
nodir: true,
816-
});
808+
const filePathsToDelete = await globFiles(filesToDelete);
817809

818810
logger.debug(
819811
"Waiting for dependencies on generated files to be freed before deleting..."
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { glob } from "glob";
2+
3+
export function globFiles(
4+
patterns: string | string[],
5+
options?: { root?: string; ignore?: string | string[] }
6+
): Promise<string[]> {
7+
return glob(patterns, { absolute: true, nodir: true, ...options });
8+
}

packages/bundler-plugin-core/src/index.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import componentNameAnnotatePlugin, {
55
import SentryCli from "@sentry/cli";
66
import { logger } from "@sentry/utils";
77
import * as fs from "fs";
8-
import { glob } from "glob";
98
import { CodeInjection, containsOnlyImports, stripQueryAndHashFromPath } from "./utils";
109

1110
/**
@@ -63,18 +62,7 @@ export function shouldSkipCodeInjection(
6362
return false;
6463
}
6564

66-
export function globFiles(outputDir: string): Promise<string[]> {
67-
return glob(
68-
["/**/*.js", "/**/*.mjs", "/**/*.cjs", "/**/*.js.map", "/**/*.mjs.map", "/**/*.cjs.map"].map(
69-
(q) => `${q}?(\\?*)?(#*)`
70-
), // We want to allow query and hashes strings at the end of files
71-
{
72-
root: outputDir,
73-
absolute: true,
74-
nodir: true,
75-
}
76-
);
77-
}
65+
export { globFiles } from "./glob";
7866

7967
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
8068
export function createComponentNameAnnotateHooks(

packages/bundler-plugin-core/test/build-plugin-manager.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
_resetDeployedReleasesForTesting,
44
} from "../src/build-plugin-manager";
55
import fs from "fs";
6-
import { glob } from "glob";
6+
import { globFiles } from "../src/glob";
77
import { prepareBundleForDebugIdUpload } from "../src/debug-id-upload";
88

99
const mockCliExecute = jest.fn();
@@ -37,10 +37,10 @@ jest.mock("@sentry/core", () => ({
3737
startSpan: jest.fn((options: unknown, callback: () => unknown) => callback()),
3838
}));
3939

40-
jest.mock("glob");
40+
jest.mock("../src/glob");
4141
jest.mock("../src/debug-id-upload");
4242

43-
const mockGlob = glob as jest.MockedFunction<typeof glob>;
43+
const mockGlobFiles = globFiles as jest.MockedFunction<typeof globFiles>;
4444
const mockPrepareBundleForDebugIdUpload = prepareBundleForDebugIdUpload as jest.MockedFunction<
4545
typeof prepareBundleForDebugIdUpload
4646
>;
@@ -302,7 +302,7 @@ describe("createSentryBuildPluginManager", () => {
302302
})
303303
);
304304
// Should not glob when prepareArtifacts is false
305-
expect(mockGlob).not.toHaveBeenCalled();
305+
expect(mockGlobFiles).not.toHaveBeenCalled();
306306
expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled();
307307
});
308308

@@ -340,7 +340,7 @@ describe("createSentryBuildPluginManager", () => {
340340
live: "rejectOnError",
341341
})
342342
);
343-
expect(mockGlob).not.toHaveBeenCalled();
343+
expect(mockGlobFiles).not.toHaveBeenCalled();
344344
expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled();
345345
});
346346

@@ -359,7 +359,7 @@ describe("createSentryBuildPluginManager", () => {
359359
await manager.uploadSourcemaps([".next"], { prepareArtifacts: false });
360360

361361
expect(mockCliUploadSourceMaps).not.toHaveBeenCalled();
362-
expect(mockGlob).not.toHaveBeenCalled();
362+
expect(mockGlobFiles).not.toHaveBeenCalled();
363363
expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled();
364364
});
365365

@@ -378,14 +378,18 @@ describe("createSentryBuildPluginManager", () => {
378378
await manager.uploadSourcemaps([".next"]);
379379

380380
expect(mockCliUploadSourceMaps).not.toHaveBeenCalled();
381-
expect(mockGlob).not.toHaveBeenCalled();
381+
expect(mockGlobFiles).not.toHaveBeenCalled();
382382
expect(mockPrepareBundleForDebugIdUpload).not.toHaveBeenCalled();
383383
});
384384

385385
it("prepares into temp folder and uploads when prepareArtifacts is true (default)", async () => {
386386
mockCliUploadSourceMaps.mockResolvedValue(undefined);
387387

388-
mockGlob.mockResolvedValue(["/app/dist/a.js", "/app/dist/a.js.map", "/app/dist/other.txt"]);
388+
mockGlobFiles.mockResolvedValue([
389+
"/app/dist/a.js",
390+
"/app/dist/a.js.map",
391+
"/app/dist/other.txt",
392+
]);
389393

390394
jest.spyOn(fs.promises, "mkdtemp").mockResolvedValue("/tmp/sentry-upload-xyz");
391395
jest.spyOn(fs.promises, "readdir").mockResolvedValue(["a.js", "a.js.map"] as never);
@@ -472,7 +476,7 @@ describe("createSentryBuildPluginManager", () => {
472476
describe("uploadSourcemaps with multiple projects", () => {
473477
beforeEach(() => {
474478
jest.clearAllMocks();
475-
mockGlob.mockResolvedValue(["/path/to/bundle.js"]);
479+
mockGlobFiles.mockResolvedValue(["/path/to/bundle.js"]);
476480
mockPrepareBundleForDebugIdUpload.mockResolvedValue(undefined);
477481
mockCliUploadSourceMaps.mockResolvedValue(undefined);
478482

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import * as fs from "fs";
2+
import * as path from "path";
3+
import * as os from "os";
4+
import { globFiles } from "../src/glob";
5+
6+
let tmpDir: string;
7+
8+
beforeEach(async () => {
9+
tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "glob-test-"));
10+
});
11+
12+
afterEach(async () => {
13+
await fs.promises.rm(tmpDir, { recursive: true, force: true });
14+
});
15+
16+
/** Helper: create a file (and any parent dirs) under tmpDir. */
17+
async function touch(...segments: string[]): Promise<string> {
18+
const filePath = path.join(tmpDir, ...segments);
19+
await fs.promises.mkdir(path.dirname(filePath), { recursive: true });
20+
await fs.promises.writeFile(filePath, "");
21+
return filePath;
22+
}
23+
24+
describe("globFiles", () => {
25+
describe("core behavior", () => {
26+
it("returns absolute paths", async () => {
27+
await touch("a.js");
28+
const result = await globFiles(path.join(tmpDir, "**/*.js"));
29+
expect(result).toHaveLength(1);
30+
expect(path.isAbsolute(result[0]!)).toBe(true);
31+
});
32+
33+
it("excludes directories (nodir)", async () => {
34+
// Create a directory that matches the glob pattern
35+
await fs.promises.mkdir(path.join(tmpDir, "subdir.js"), { recursive: true });
36+
await touch("real.js");
37+
const result = await globFiles(path.join(tmpDir, "**/*.js"));
38+
expect(result).toEqual([path.join(tmpDir, "real.js")]);
39+
});
40+
41+
it("returns [] for no matches", async () => {
42+
await touch("a.txt");
43+
const result = await globFiles(path.join(tmpDir, "**/*.js"));
44+
expect(result).toEqual([]);
45+
});
46+
47+
it("returns [] for nonexistent pattern path", async () => {
48+
const result = await globFiles(path.join(tmpDir, "nonexistent/**/*.js"));
49+
expect(result).toEqual([]);
50+
});
51+
52+
it("matches deeply nested files", async () => {
53+
const filePath = await touch("a", "b", "c", "deep.js");
54+
const result = await globFiles(path.join(tmpDir, "**/*.js"));
55+
expect(result).toEqual([filePath]);
56+
});
57+
58+
it("works with a string pattern", async () => {
59+
await touch("single.js");
60+
const result = await globFiles(path.join(tmpDir, "*.js"));
61+
expect(result).toHaveLength(1);
62+
});
63+
64+
it("works with an array of patterns", async () => {
65+
const jsFile = await touch("a.js");
66+
const mapFile = await touch("a.js.map");
67+
await touch("b.css");
68+
69+
const result = await globFiles([
70+
path.join(tmpDir, "**/*.js"),
71+
path.join(tmpDir, "**/*.js.map"),
72+
]);
73+
result.sort();
74+
expect(result).toEqual([jsFile, mapFile].sort());
75+
});
76+
});
77+
78+
describe("root option", () => {
79+
it("scopes results to root directory", async () => {
80+
await touch("file.js");
81+
// Patterns starting with / are resolved relative to root
82+
const result = await globFiles("/**/*.js", { root: tmpDir });
83+
expect(result).toEqual([path.join(tmpDir, "file.js")]);
84+
});
85+
});
86+
87+
describe("ignore option", () => {
88+
it("excludes files matching ignore string pattern", async () => {
89+
await touch("keep.js");
90+
await touch("node_modules", "dep.js");
91+
92+
const result = await globFiles(path.join(tmpDir, "**/*.js"), {
93+
ignore: path.join(tmpDir, "node_modules/**"),
94+
});
95+
expect(result).toEqual([path.join(tmpDir, "keep.js")]);
96+
});
97+
98+
it("excludes files matching ignore array patterns", async () => {
99+
await touch("keep.js");
100+
await touch("node_modules", "dep.js");
101+
await touch("dist", "bundle.js");
102+
103+
const result = await globFiles(path.join(tmpDir, "**/*.js"), {
104+
ignore: [path.join(tmpDir, "node_modules/**"), path.join(tmpDir, "dist/**")],
105+
});
106+
expect(result).toEqual([path.join(tmpDir, "keep.js")]);
107+
});
108+
});
109+
110+
describe("rollup JS/map patterns", () => {
111+
const JS_AND_MAP_PATTERNS = [
112+
"/**/*.js",
113+
"/**/*.mjs",
114+
"/**/*.cjs",
115+
"/**/*.js.map",
116+
"/**/*.mjs.map",
117+
"/**/*.cjs.map",
118+
].map((q) => `${q}?(\\?*)?(#*)`);
119+
120+
it("matches .js, .mjs, .cjs and their .map variants", async () => {
121+
const files = await Promise.all([
122+
touch("a.js"),
123+
touch("b.mjs"),
124+
touch("c.cjs"),
125+
touch("a.js.map"),
126+
touch("b.mjs.map"),
127+
touch("c.cjs.map"),
128+
]);
129+
130+
const result = await globFiles(JS_AND_MAP_PATTERNS, { root: tmpDir });
131+
result.sort();
132+
expect(result).toEqual(files.sort());
133+
});
134+
135+
it("does NOT match .css, .ts, .json, etc.", async () => {
136+
await touch("style.css");
137+
await touch("types.ts");
138+
await touch("data.json");
139+
await touch("readme.md");
140+
141+
const result = await globFiles(JS_AND_MAP_PATTERNS, { root: tmpDir });
142+
expect(result).toEqual([]);
143+
});
144+
145+
it("works in nested subdirectories", async () => {
146+
const files = await Promise.all([
147+
touch("src", "deep", "a.js"),
148+
touch("src", "deep", "a.js.map"),
149+
]);
150+
151+
const result = await globFiles(JS_AND_MAP_PATTERNS, { root: tmpDir });
152+
result.sort();
153+
expect(result).toEqual(files.sort());
154+
});
155+
156+
it("returns [] for empty directory", async () => {
157+
const result = await globFiles(JS_AND_MAP_PATTERNS, { root: tmpDir });
158+
expect(result).toEqual([]);
159+
});
160+
});
161+
});

packages/rollup-plugin/src/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,15 @@ export function _rollupPluginInternal(
208208
if (sourcemapsEnabled && options.sourcemaps?.disable !== "disable-upload") {
209209
if (outputOptions.dir) {
210210
const outputDir = outputOptions.dir;
211-
const buildArtifacts = await globFiles(outputDir);
211+
const JS_AND_MAP_PATTERNS = [
212+
"/**/*.js",
213+
"/**/*.mjs",
214+
"/**/*.cjs",
215+
"/**/*.js.map",
216+
"/**/*.mjs.map",
217+
"/**/*.cjs.map",
218+
].map((q) => `${q}?(\\?*)?(#*)`); // We want to allow query and hash strings at the end of files
219+
const buildArtifacts = await globFiles(JS_AND_MAP_PATTERNS, { root: outputDir });
212220
await upload(buildArtifacts);
213221
} else if (outputOptions.file) {
214222
await upload([outputOptions.file]);

0 commit comments

Comments
 (0)