Skip to content

Remove commondir dependency, inline as commonParentDirectory()#7176

Merged
ryancbahan merged 1 commit intomainfrom
04-02-remove-commondir-dep
Apr 3, 2026
Merged

Remove commondir dependency, inline as commonParentDirectory()#7176
ryancbahan merged 1 commit intomainfrom
04-02-remove-commondir-dep

Conversation

@ryancbahan
Copy link
Copy Markdown
Contributor

@ryancbahan ryancbahan commented Apr 2, 2026

Summary

  • Replace the unmaintained commondir package (~2014, 21 lines) with an inlined commonParentDirectory() function in path.ts
  • Remove both commondir and @types/commondir dependencies
  • Add parity tests covering the original package's test suite (shared prefixes, root divergence, Windows paths, etc.)

@ryancbahan ryancbahan changed the title Remove commondir dependency, inline as simple helper Remove commondir dependency, inline as commonParentDirectory() Apr 2, 2026
The commondir package is a tiny, unmaintained (~2014) utility. Inline
its logic as an exported commonParentDirectory() function in path.ts,
removing both the runtime dep and @types/commondir. Added parity tests
covering the original package's test cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan marked this pull request as ready for review April 2, 2026 23:19
@ryancbahan ryancbahan requested a review from a team as a code owner April 2, 2026 23:19
@ryancbahan ryancbahan force-pushed the 04-02-remove-commondir-dep branch from 9c0de49 to 8e7cc63 Compare April 2, 2026 23:19
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/path.d.ts
 import type { URL } from 'url';
 /**
  * Joins a list of paths together.
  *
  * @param paths - Paths to join.
  * @returns Joined path.
  */
 export declare function joinPath(...paths: string[]): string;
 /**
  * Normalizes a path.
  *
  * @param path - Path to normalize.
  * @returns Normalized path.
  */
 export declare function normalizePath(path: string): string;
 /**
  * Resolves a list of paths together.
  *
  * @param paths - Paths to resolve.
  * @returns Resolved path.
  */
 export declare function resolvePath(...paths: string[]): string;
 /**
  * Returns the relative path from one path to another.
  *
  * @param from - Path to resolve from.
  * @param to - Path to resolve to.
  * @returns Relative path.
  */
 export declare function relativePath(from: string, to: string): string;
 /**
  * Returns whether the path is absolute.
  *
  * @param path - Path to check.
  * @returns Whether the path is absolute.
  */
 export declare function isAbsolutePath(path: string): boolean;
 /**
  * Returns the directory name of a path.
  *
  * @param path - Path to get the directory name of.
  * @returns Directory name.
  */
 export declare function dirname(path: string): string;
 /**
  * Returns the base name of a path.
  *
  * @param path - Path to get the base name of.
  * @param ext - Optional extension to remove from the result.
  * @returns Base name.
  */
 export declare function basename(path: string, ext?: string): string;
 /**
  * Returns the extension of the path.
  *
  * @param path - Path to get the extension of.
  * @returns Extension.
  */
 export declare function extname(path: string): string;
 /**
  * Parses a path into its components (root, dir, base, ext, name).
  *
  * @param path - Path to parse.
  * @returns Parsed path object.
  */
 export declare function parsePath(path: string): {
     root: string;
     dir: string;
     base: string;
     ext: string;
     name: string;
 };
 /**
+ * Returns the longest common parent directory of two absolute paths.
+ *
+ * @param first - First absolute path.
+ * @param second - Second absolute path.
+ * @returns The common parent directory, or '/' if they share only the root.
+ */
+export declare function commonParentDirectory(first: string, second: string): string;
+/**
  * Given an absolute filesystem path, it makes it relative to
  * the current working directory. This is useful when logging paths
  * to allow the users to click on the file and let the OS open it
  * in the editor of choice.
  *
  * @param path - Path to relativize.
  * @param dir - Current working directory.
  * @returns Relativized path.
  */
 export declare function relativizePath(path: string, dir?: string): string;
 /**
  * Given 2 paths, it returns whether the second path is a subpath of the first path.
  *
  * @param mainPath - The main path.
  * @param subpath - The subpath.
  * @returns Whether the subpath is a subpath of the main path.
  */
 export declare function isSubpath(mainPath: string, subpath: string): boolean;
 /**
  * Given a module's import.meta.url it returns the directory containing the module.
  *
  * @param moduleURL - The value of import.meta.url in the context of the caller module.
  * @returns The path to the directory containing the caller module.
  */
 export declare function moduleDirectory(moduleURL: string | URL): string;
 /**
  * When running a script using `npm run`, something interesting happens. If the current
  * folder does not have a `package.json` or a `node_modules` folder, npm will traverse
  * the directory tree upwards until it finds one. Then it will run the script and set
  * `process.cwd()` to that folder, while the actual path is stored in the INIT_CWD
  * environment variable (see here: https://docs.npmjs.com/cli/v9/commands/npm-run-script#description).
  *
  * @returns The path to the current working directory.
  */
 export declare function cwd(): string;
 /**
  * Tries to get the value of the `--path` argument, if provided.
  *
  * @param argv - The arguments to search for the `--path` argument.
  * @returns The value of the `--path` argument, if provided.
  */
 export declare function sniffForPath(argv?: string[]): string | undefined;
 /**
  * Returns whether the `--json` or `-j` flags are present in the arguments.
  *
  * @param argv - The arguments to search for the `--json` and `-j` flags.
  * @returns Whether the `--json` or `-j` flag is present in the arguments.
  */
 export declare function sniffForJson(argv?: string[]): boolean;
 /**
  * Removes any `..` traversal segments from a relative path and calls `warn`
  * if any were stripped. Normal `..` that cancel out within the path (e.g.
  * `foo/../bar` → `bar`) are collapsed but never allowed to escape the root.
  * Both `/` and `\` are treated as separators for cross-platform safety.
  *
  * @param input - The relative path to sanitize.
  * @param warn - Called with a human-readable warning when traversal segments are removed.
  * @returns The sanitized path (may be an empty string if all segments were traversal).
  */
 export declare function sanitizeRelativePath(input: string, warn: (msg: string) => void): string;

Copy link
Copy Markdown
Contributor

@dmerand dmerand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It might be cleaner to avoid adding this to the public cli-kit API surface, and refactor tests to relativizePath. A bit more indirect but has no public-facing change.

Copy link
Copy Markdown
Contributor Author

@dmerand hm, I hear you, but it would break with the pattern of putting node utils into cli-kit. Let's maybe chat about this?

@ryancbahan ryancbahan added this pull request to the merge queue Apr 3, 2026
@dmerand
Copy link
Copy Markdown
Contributor

dmerand commented Apr 3, 2026

I just mean not exporting the function.

Copy link
Copy Markdown
Contributor Author

oh, apologies -- i misread. that's fair. i'll follow up with a pr to do that.

Merged via the queue into main with commit 573ad45 Apr 3, 2026
24 checks passed
@ryancbahan ryancbahan deleted the 04-02-remove-commondir-dep branch April 3, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants