Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add root node_modules to import commands when running pnpm #1643

Merged
merged 5 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion packages/cli/src/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ function formatFilepath(filepath) {
return filepath;
}

export function getSiblings(root) {
const siblings = [path.join(root, '..')];

// Check if Percy CLI is installed using '.pnpm' by searching
// ffor the .pnpm folder in the root path
const nodeModulesIndex = root.indexOf('.pnpm');
if (nodeModulesIndex !== -1) {
// add the parent directory of the .pnpm and append /@percy
siblings.push(path.join(root.substring(0, nodeModulesIndex), '@percy'));
}

return siblings;
}

// Imports and returns compatibile CLI commands from various sources
export async function importCommands() {
let root = path.resolve(url.fileURLToPath(import.meta.url), '../..');
Expand All @@ -109,7 +123,7 @@ export async function importCommands() {
// find included dependencies
root,
// find potential sibling packages
path.join(root, '..'),
...getSiblings(root),
// find any current project dependencies
process.cwd()
]), async (roots, dir) => {
Expand Down
29 changes: 28 additions & 1 deletion packages/cli/test/commands.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from 'path';
import { logger, mockfs, fs } from '@percy/cli-command/test/helpers';
import { mockModuleCommands, mockPnpCommands, mockLegacyCommands } from './helpers.js';
import { importCommands } from '../src/commands.js';
import { importCommands, getSiblings } from '../src/commands.js';

describe('CLI commands', () => {
beforeEach(async () => {
Expand Down Expand Up @@ -213,3 +213,30 @@ describe('CLI commands', () => {
});
});
});

describe('getSiblings', () => {
it('returns the parent directory for a path without .pnpm', () => {
const root = '/path/to/project';
const expected = [path.join(root, '..')];
expect(getSiblings(root)).toEqual(expected);
});

it('returns the parent directory and @percy path for a path with .pnpm', () => {
const root = '/path/to/.pnpm/project';
const expected = [
path.join(root, '..'),
path.join('/path/to', '@percy')
];
expect(getSiblings(root)).toEqual(expected);
});

it('handles paths with .pnpm but no node_modules correctly', () => {
// This test ensures that the function's logic for manipulating the path works even without a direct node_modules relationship.
const root = '/path/with/.pnpm/and/no/node_modules';
const expected = [
path.join(root, '..'),
path.join('/path/with', '@percy')
];
expect(getSiblings(root)).toEqual(expected);
});
});
Loading