Skip to content

Commit ba933fc

Browse files
committed
fix remove-unused-fields command
- make it use rescript-tools from the installed rescript package instead of relying on the outdated @rescript/tools package - get the artifact directory from relay config - do not analyze files in relay config `excludes`
1 parent 049414f commit ba933fc

6 files changed

Lines changed: 250 additions & 14 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import fs from "fs";
2+
import os from "os";
3+
import path from "path";
4+
import {
5+
findSourceFiles,
6+
getReanalyzeCwd,
7+
getRescriptToolsCommand,
8+
} from "../fileUtils";
9+
10+
describe("source discovery", () => {
11+
it("honors relay excludes when finding source files", async () => {
12+
const originalCwd = process.cwd();
13+
const root = fs.mkdtempSync(
14+
path.join(os.tmpdir(), "rescript-relay-source-discovery-")
15+
);
16+
const projectRoot = path.join(root, "packages", "web");
17+
18+
try {
19+
fs.mkdirSync(path.join(projectRoot, "src"), { recursive: true });
20+
fs.mkdirSync(path.join(projectRoot, "lib", "ocaml"), {
21+
recursive: true,
22+
});
23+
fs.writeFileSync(
24+
path.join(projectRoot, "src", "OrganizationSettings.res"),
25+
"let actualSource = true\n",
26+
{ encoding: "utf-8" }
27+
);
28+
fs.writeFileSync(
29+
path.join(projectRoot, "lib", "ocaml", "OrganizationSettings.res"),
30+
"let compiledCopy = true\n",
31+
{ encoding: "utf-8" }
32+
);
33+
34+
process.chdir(projectRoot);
35+
36+
const sourceFiles = await findSourceFiles(
37+
["OrganizationSettings.res"],
38+
".",
39+
["**/lib/**"]
40+
);
41+
42+
expect(sourceFiles.map((file) => fs.realpathSync(file))).toEqual([
43+
fs.realpathSync(
44+
path.join(projectRoot, "src", "OrganizationSettings.res")
45+
),
46+
]);
47+
} finally {
48+
process.chdir(originalCwd);
49+
fs.rmSync(root, { recursive: true, force: true });
50+
}
51+
});
52+
53+
it("uses the ancestor ReScript analysis root and ReScript tools wrapper", () => {
54+
const root = fs.mkdtempSync(
55+
path.join(os.tmpdir(), "rescript-relay-source-discovery-")
56+
);
57+
const webRoot = path.join(root, "packages", "web");
58+
59+
try {
60+
fs.mkdirSync(path.join(root, "lib", "bs"), { recursive: true });
61+
fs.mkdirSync(path.join(root, "node_modules", "rescript", "cli"), {
62+
recursive: true,
63+
});
64+
fs.mkdirSync(webRoot, { recursive: true });
65+
fs.writeFileSync(
66+
path.join(root, "lib", "bs", ".sourcedirs.json"),
67+
"[]\n",
68+
{ encoding: "utf-8" }
69+
);
70+
fs.writeFileSync(
71+
path.join(
72+
root,
73+
"node_modules",
74+
"rescript",
75+
"cli",
76+
"rescript-tools.js"
77+
),
78+
"",
79+
{ encoding: "utf-8" }
80+
);
81+
82+
expect(getReanalyzeCwd(webRoot)).toBe(root);
83+
expect(getRescriptToolsCommand(webRoot)).toEqual({
84+
command: process.execPath,
85+
args: [
86+
path.join(
87+
root,
88+
"node_modules",
89+
"rescript",
90+
"cli",
91+
"rescript-tools.js"
92+
),
93+
"reanalyze",
94+
"-dce",
95+
],
96+
});
97+
} finally {
98+
fs.rmSync(root, { recursive: true, force: true });
99+
}
100+
});
101+
});

packages/rescript-relay-cli/cliUtils.ts

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ export const extractGraphQLSourceFromReScript = makeExtractTagsFromSource(
5050
rescriptGraphQLTagsRegexp
5151
);
5252

53+
const escapeRegExp = (str: string): string =>
54+
str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
55+
5356
export function prettify(str: string): string {
5457
return (
5558
format(str, {
@@ -410,16 +413,49 @@ export interface IFragmentRepresentationWithSourceLocation
410413
sourceLocation: string;
411414
}
412415

413-
const graphqlNameRegexp = new RegExp(
414-
/(?<=__generated__\/)[A-Za-z_0-9]+(?=_graphql\.res)/g
415-
);
416416
const fieldPathRegexp = new RegExp(
417417
/(?<=Types\.(fragment|response)[_.])[A-Za-z_.0-9]+(?= )/g
418418
);
419419

420420
const filterRegexp = new RegExp(/Types\.(fragment|response)/g);
421421

422-
export const processReanalyzeOutput = (output: string) => {
422+
const extractGeneratedFileNameFromReanalyzeOutput = (
423+
output: string,
424+
artifactDirectoryLocation?: string
425+
): string | null => {
426+
const normalizedOutput = output.replace(/\\/g, "/");
427+
428+
if (artifactDirectoryLocation != null) {
429+
const normalizedArtifactDirectoryLocation = artifactDirectoryLocation
430+
.replace(/\\/g, "/")
431+
.replace(/\/$/, "");
432+
const artifactRegexp = new RegExp(
433+
`${escapeRegExp(
434+
normalizedArtifactDirectoryLocation
435+
)}/([A-Za-z_0-9]+_graphql\\.res)(?:"|:)`
436+
);
437+
const explicitArtifactMatch = normalizedOutput.match(artifactRegexp)?.[1];
438+
439+
if (explicitArtifactMatch != null) {
440+
return explicitArtifactMatch;
441+
}
442+
}
443+
444+
return (
445+
normalizedOutput.match(
446+
/(?:__generated__|__relay__)\/([A-Za-z_0-9]+_graphql\.res)(?:"|:)/
447+
)?.[1] ?? null
448+
);
449+
};
450+
451+
export const processReanalyzeOutput = (
452+
output: string,
453+
{
454+
artifactDirectoryLocation,
455+
}: {
456+
artifactDirectoryLocation?: string;
457+
} = {}
458+
) => {
423459
const processed = output
424460
// Parse reanalyze output
425461
.split(/\n\n/g)
@@ -435,9 +471,12 @@ export const processReanalyzeOutput = (output: string) => {
435471

436472
const type = curr.includes("Types.fragment") ? "fragment" : "query";
437473

438-
const graphqlName = curr.match(graphqlNameRegexp)?.[0];
439-
const fileName =
440-
graphqlName == null ? null : `${graphqlName}_graphql.res`;
474+
const fileName = extractGeneratedFileNameFromReanalyzeOutput(
475+
curr,
476+
artifactDirectoryLocation
477+
);
478+
const graphqlName =
479+
fileName == null ? null : fileName.replace(/_graphql\.res$/, "");
441480
const fieldPath = curr.match(fieldPathRegexp)?.[0];
442481

443482
// We only care about queries

packages/rescript-relay-cli/commands/debugCommand.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
findAllSourceFilesFromGeneratedFiles,
77
findSourceFiles,
88
getSrcCwd,
9+
getRelayExcludes,
910
} from "../fileUtils";
1011

1112
export const addDebugCommand = (program: Command) => {
@@ -41,7 +42,11 @@ export const addDebugCommand = (program: Command) => {
4142

4243
console.log("Looking up source files..\n");
4344

44-
const sourceFiles = await findSourceFiles(files, relayConfig.src);
45+
const sourceFiles = await findSourceFiles(
46+
files,
47+
relayConfig.src,
48+
getRelayExcludes(relayConfig)
49+
);
4550

4651
console.log(
4752
`Found ${

packages/rescript-relay-cli/commands/formatGraphQLCommand.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
findSourceFiles,
99
getAllGeneratedFiles,
1010
getRelayArtifactDirectoryLocation,
11+
getRelayExcludes,
1112
loadRelayConfig,
1213
} from "../fileUtils";
1314
import { formatOperationsInDocument } from "../formatUtils";
@@ -38,7 +39,11 @@ export const addFormatGraphQLCommands = (program: Command) => {
3839
sourcesToFind.length
3940
)}.`;
4041

41-
const files = await findSourceFiles(sourcesToFind, relayConfig.src);
42+
const files = await findSourceFiles(
43+
sourcesToFind,
44+
relayConfig.src,
45+
getRelayExcludes(relayConfig)
46+
);
4247

4348
const formatSuccesses = await Promise.all(
4449
files.map(async (filePath) => {

packages/rescript-relay-cli/commands/removeUnusedFieldsCommand.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ import {
2525
getRelayArtifactDirectoryLocation,
2626
sourceLocExtractor,
2727
findSourceFiles,
28+
getRelayExcludes,
29+
getReanalyzeCwd,
30+
getRescriptToolsCommand,
2831
} from "../fileUtils";
2932

3033
export const addRemoveUnusedFieldsCommand = (program: Command) => {
@@ -52,10 +55,14 @@ export const addRemoveUnusedFieldsCommand = (program: Command) => {
5255
const relayConfig = loadRelayConfig();
5356
const artifactDirectoryLocation =
5457
getRelayArtifactDirectoryLocation(relayConfig);
58+
const reanalyzeCwd = getReanalyzeCwd(process.cwd());
59+
const reanalyzeCommand = getRescriptToolsCommand(reanalyzeCwd);
5560

5661
const spinner = ora("Analyzing ReScript project").start();
5762

58-
const p = cp.spawn("npx", ["--yes", "@rescript/tools", "reanalyze", "-dce"]);
63+
const p = cp.spawn(reanalyzeCommand.command, reanalyzeCommand.args, {
64+
cwd: reanalyzeCwd,
65+
});
5966

6067
if (p.stdout == null) {
6168
console.error("Something went wrong.");
@@ -86,7 +93,9 @@ export const addRemoveUnusedFieldsCommand = (program: Command) => {
8693

8794
p.on("close", async () => {
8895
spinner.text = "Analyzing GraphQL usage";
89-
const processed = processReanalyzeOutput(data);
96+
const processed = processReanalyzeOutput(data, {
97+
artifactDirectoryLocation,
98+
});
9099

91100
if (debug) {
92101
console.log(
@@ -150,7 +159,8 @@ export const addRemoveUnusedFieldsCommand = (program: Command) => {
150159

151160
const files = await findSourceFiles(
152161
sourcesToFind.map((s) => s.path),
153-
relayConfig.src
162+
relayConfig.src,
163+
getRelayExcludes(relayConfig)
154164
);
155165

156166
const filesWithInfo = files.map((absoluteFilePath) => {

packages/rescript-relay-cli/fileUtils.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import ora from "ora";
33
import path from "path";
44
import fs from "fs";
55
import { cosmiconfigSync } from "cosmiconfig";
6-
import { Config } from "relay-compiler/lib/bin/RelayCompilerMain";
6+
import type { Config } from "relay-compiler/lib/bin/RelayCompilerMain";
77

88
const config = cosmiconfigSync("relay", {
99
searchPlaces: [
@@ -97,14 +97,90 @@ export const findAllSourceFilesFromGeneratedFiles = async (
9797
export const getSrcCwd = (src: string) =>
9898
path.resolve(path.join(process.cwd(), src));
9999

100-
export const findSourceFiles = async (fileNames: string[], src: string) => {
100+
export const getRelayExcludes = (
101+
relayConfig: Partial<Config> & { excludes?: string[] }
102+
): string[] => relayConfig.excludes ?? relayConfig.exclude ?? [];
103+
104+
const findAncestorContaining = (startDir: string, relativeFilePath: string) => {
105+
let currentDir = path.resolve(startDir);
106+
107+
while (true) {
108+
const candidate = path.join(currentDir, relativeFilePath);
109+
110+
if (fs.existsSync(candidate)) {
111+
return currentDir;
112+
}
113+
114+
const parentDir = path.dirname(currentDir);
115+
116+
if (parentDir === currentDir) {
117+
return null;
118+
}
119+
120+
currentDir = parentDir;
121+
}
122+
};
123+
124+
export const getReanalyzeCwd = (startDir: string) =>
125+
findAncestorContaining(startDir, path.join("lib", "bs", ".sourcedirs.json")) ??
126+
startDir;
127+
128+
export const getRescriptToolsCommand = (startDir: string) => {
129+
const rescriptToolsRoot = findAncestorContaining(
130+
startDir,
131+
path.join("node_modules", "rescript", "cli", "rescript-tools.js")
132+
);
133+
134+
if (rescriptToolsRoot != null) {
135+
return {
136+
command: process.execPath,
137+
args: [
138+
path.join(
139+
rescriptToolsRoot,
140+
"node_modules",
141+
"rescript",
142+
"cli",
143+
"rescript-tools.js"
144+
),
145+
"reanalyze",
146+
"-dce",
147+
],
148+
};
149+
}
150+
151+
const binName =
152+
process.platform === "win32" ? "rescript-tools.cmd" : "rescript-tools";
153+
const packageRoot = findAncestorContaining(
154+
startDir,
155+
path.join("node_modules", ".bin", binName)
156+
);
157+
158+
if (packageRoot != null) {
159+
return {
160+
command: path.join(packageRoot, "node_modules", ".bin", binName),
161+
args: ["reanalyze", "-dce"],
162+
};
163+
}
164+
165+
return {
166+
command: "npx",
167+
args: ["--yes", "@rescript/tools", "reanalyze", "-dce"],
168+
};
169+
};
170+
171+
export const findSourceFiles = async (
172+
fileNames: string[],
173+
src: string,
174+
excludes: string[] = []
175+
) => {
101176
const cwd = getSrcCwd(src);
102177

103178
const files = await glob(
104179
fileNames.map((name) => `**/${name}`),
105180
{
106181
cwd,
107182
absolute: true,
183+
ignore: excludes,
108184
onlyFiles: true,
109185
}
110186
);

0 commit comments

Comments
 (0)