Skip to content

Commit ef94d1c

Browse files
authored
Support absolute targetPackagePath independent of cwd (#204)
`targetPackagePath` was resolved with `path.join(process.cwd(), targetPackagePath)` and the workspace root pinned to `process.cwd()`, so callers had to guarantee the process runs at the workspace root. An absolute path produced a broken join. An absolute `targetPackagePath` now targets that directory directly, and the workspace root is resolved the same way as the no-`targetPackagePath` case: from the `workspaceRoot` setting, or auto-detected by walking upward from the target package directory. Relative paths keep their exact existing semantics, so this is purely additive — absolute paths were previously a broken input nobody could depend on. Motivation: the firebase-tools PR upstreaming the isolate integration (firebase/firebase-tools#10996) got review feedback that its isolate call breaks when the CLI runs from a subdirectory of the project. With this change firebase-tools can pass its already-resolved absolute source directory and be correct from any cwd, without a `process.chdir` workaround. Decisions: - Gated the new behavior on `path.isAbsolute` rather than changing relative-path semantics, to stay non-breaking for existing configs. - With an absolute path, `workspaceRoot` is honored instead of ignored (it was only ignored before because cwd was assumed to be the root).
1 parent f1c29f9 commit ef94d1c

3 files changed

Lines changed: 132 additions & 12 deletions

File tree

docs/configuration.md

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,14 @@ Only when you decide to place the isolate configuration in the root of the
130130
monorepo, you use this setting to point it to the target you want to isolate,
131131
e.g. `./packages/my-firebase-package`.
132132

133-
If this option is used the `workspaceRoot` setting will be ignored and assumed
134-
to be the current working directory.
133+
If this option is set to a relative path, the `workspaceRoot` setting will be
134+
ignored and the workspace root is assumed to be the current working directory.
135+
136+
The path can also be absolute, which makes isolation independent of the
137+
current working directory. In that case the workspace root is resolved the
138+
same way as when `targetPackagePath` is omitted: from the `workspaceRoot`
139+
setting, or otherwise auto-detected by walking upward from the target package
140+
directory.
135141

136142
### tsconfigPath
137143

@@ -168,5 +174,6 @@ You only need to set this explicitly when auto-detection fails, for example
168174
when the target package is nested more than a few levels deep inside the
169175
workspace.
170176

171-
When you use the `targetPackagePath` option, this setting is ignored and the
172-
workspace root is assumed to be the current working directory.
177+
When you set the `targetPackagePath` option to a relative path, this setting
178+
is ignored and the workspace root is assumed to be the current working
179+
directory. With an absolute `targetPackagePath` this setting is honored.

src/lib/config.test.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import fs from "fs-extra";
22
import path from "node:path";
33
import os from "node:os";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5-
import { defineConfig, loadConfigFromFile } from "./config";
5+
import {
6+
defineConfig,
7+
type IsolateConfigResolved,
8+
loadConfigFromFile,
9+
resolveWorkspacePaths,
10+
} from "./config";
611

712
/** Shared mock logger instance so assertions can check calls. */
813
const mockLogger = {
@@ -161,3 +166,99 @@ describe("defineConfig", () => {
161166
expect(result).toBe(input);
162167
});
163168
});
169+
170+
describe("resolveWorkspacePaths", () => {
171+
/** The required config fields; path-related options are added per test. */
172+
const baseConfig: IsolateConfigResolved = {
173+
includeDevDependencies: false,
174+
isolateDirName: "isolate",
175+
logLevel: "info",
176+
tsconfigPath: "./tsconfig.json",
177+
forceNpm: false,
178+
};
179+
180+
let tempDir: string;
181+
let originalCwd: string;
182+
183+
beforeEach(async () => {
184+
originalCwd = process.cwd();
185+
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "isolate-paths-test-"));
186+
/**
187+
* Resolve symlinks (macOS tmpdir) so comparisons against process.cwd()
188+
* and detected roots are stable.
189+
*/
190+
tempDir = await fs.realpath(dir);
191+
/** Bound upward workspace detection to the fixture. */
192+
await fs.mkdir(path.join(tempDir, ".git"));
193+
});
194+
195+
afterEach(async () => {
196+
process.chdir(originalCwd);
197+
await fs.remove(tempDir);
198+
});
199+
200+
it("treats a relative targetPackagePath as cwd-relative with cwd as workspace root", () => {
201+
process.chdir(tempDir);
202+
203+
const result = resolveWorkspacePaths({
204+
...baseConfig,
205+
targetPackagePath: "./packages/functions",
206+
});
207+
208+
expect(result.targetPackageDir).toBe(
209+
path.join(tempDir, "./packages/functions"),
210+
);
211+
expect(result.workspaceRootDir).toBe(tempDir);
212+
});
213+
214+
it("uses an absolute targetPackagePath independent of cwd and auto-detects the workspace root", async () => {
215+
await fs.writeFile(
216+
path.join(tempDir, "pnpm-workspace.yaml"),
217+
"packages:\n - packages/*\n",
218+
);
219+
const targetPackageDir = path.join(tempDir, "packages", "functions");
220+
await fs.mkdirp(targetPackageDir);
221+
await fs.writeFile(
222+
path.join(targetPackageDir, "package.json"),
223+
'{"name":"functions"}',
224+
);
225+
/** Run from an unrelated directory to prove cwd independence. */
226+
process.chdir(os.tmpdir());
227+
228+
const result = resolveWorkspacePaths({
229+
...baseConfig,
230+
targetPackagePath: targetPackageDir,
231+
});
232+
233+
expect(result.targetPackageDir).toBe(targetPackageDir);
234+
expect(result.workspaceRootDir).toBe(tempDir);
235+
});
236+
237+
it("honors the workspaceRoot setting for an absolute targetPackagePath", async () => {
238+
const targetPackageDir = path.join(tempDir, "packages", "functions");
239+
await fs.mkdirp(targetPackageDir);
240+
process.chdir(os.tmpdir());
241+
242+
const result = resolveWorkspacePaths({
243+
...baseConfig,
244+
targetPackagePath: targetPackageDir,
245+
workspaceRoot: "../..",
246+
});
247+
248+
expect(result.targetPackageDir).toBe(targetPackageDir);
249+
expect(result.workspaceRootDir).toBe(path.join(targetPackageDir, "../.."));
250+
});
251+
252+
it("throws for an absolute targetPackagePath when no workspace root is found", async () => {
253+
const targetPackageDir = path.join(tempDir, "packages", "functions");
254+
await fs.mkdirp(targetPackageDir);
255+
process.chdir(os.tmpdir());
256+
257+
expect(() =>
258+
resolveWorkspacePaths({
259+
...baseConfig,
260+
targetPackagePath: targetPackageDir,
261+
}),
262+
).toThrow(/Failed to auto-detect monorepo workspace root/);
263+
});
264+
});

src/lib/config.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ export type IsolateConfigResolved = {
1212
includeDevDependencies: boolean;
1313
isolateDirName: string;
1414
logLevel: LogLevel;
15+
/**
16+
* Path to the package that should be isolated. A relative path is resolved
17+
* against the current working directory, which is then assumed to be the
18+
* workspace root. An absolute path makes isolation independent of the
19+
* current working directory; the workspace root is then taken from
20+
* `workspaceRoot` or auto-detected by walking upward from the target
21+
* package directory. When omitted, the current working directory is the
22+
* target package.
23+
*/
1524
targetPackagePath?: string;
1625
tsconfigPath: string;
1726
workspacePackages?: string[];
@@ -175,18 +184,21 @@ function validateConfig(config: IsolateConfig) {
175184

176185
/**
177186
* Resolve the target package directory and workspace root directory from the
178-
* configuration. When targetPackagePath is set, the config is assumed to live
179-
* at the workspace root. Otherwise it lives in the target package directory.
180-
*
181-
* When `workspaceRoot` is not explicitly set, auto-detect the monorepo root by
182-
* walking upward from the target package directory.
187+
* configuration. When targetPackagePath is a relative path, it is resolved
188+
* against the current working directory and the config is assumed to live at
189+
* the workspace root. When targetPackagePath is an absolute path, the current
190+
* working directory is irrelevant and the workspace root is resolved like it
191+
* is for the no-targetPackagePath case: from the `workspaceRoot` setting, or
192+
* otherwise auto-detected by walking upward from the target package directory.
183193
*/
184194
export function resolveWorkspacePaths(config: IsolateConfigResolved) {
185195
const targetPackageDir = config.targetPackagePath
186-
? path.join(process.cwd(), config.targetPackagePath)
196+
? path.isAbsolute(config.targetPackagePath)
197+
? config.targetPackagePath
198+
: path.join(process.cwd(), config.targetPackagePath)
187199
: process.cwd();
188200

189-
if (config.targetPackagePath) {
201+
if (config.targetPackagePath && !path.isAbsolute(config.targetPackagePath)) {
190202
return { targetPackageDir, workspaceRootDir: process.cwd() };
191203
}
192204

0 commit comments

Comments
 (0)