Skip to content

Commit 34d60e4

Browse files
hlenkeclaudesorccu
authored
feat(cli): auto-include pnpm patches directory in Playwright bundles (#1273)
* feat(cli): auto-include pnpm patches directory in Playwright bundles When using pnpm, the CLI now automatically includes the `patches/` directory in Playwright check bundles if it exists, preventing `pnpm install` failures at runtime when patched dependencies are referenced in the lockfile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve require-await lint error in test mock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: use async fs operations in getAutoIncludes tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: remove unnecessary patches dir existence check from getAutoIncludes The glob pattern harmlessly matches nothing when the directory doesn't exist, so the statSync check was redundant. This also removes the basePath parameter and simplifies the tests to pure logic checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: narrow auto-include pattern to patches/*.patch More precise than patches/** — only includes actual patch files, avoiding accidentally bundling unrelated files in the directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve include patterns to absolute paths in getAutoIncludes Prevents adding duplicate patches include when the user specifies the pattern with a relative prefix (./patches/**) or absolute path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use platform-appropriate paths in getAutoIncludes tests path.resolve ensures basePath is valid on Windows where /project is not treated as an absolute path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Simo Kinnunen <simo@checklyhq.com>
1 parent f9d203f commit 34d60e4

File tree

2 files changed

+74
-2
lines changed

2 files changed

+74
-2
lines changed

packages/cli/src/services/__tests__/util.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import {
55
pathToPosix,
66
isFileSync,
77
getPlaywrightVersionFromPackage,
8+
getAutoIncludes,
89
} from '../util'
10+
import { PackageManager } from '../check-parser/package-files/package-manager'
911

1012
describe('util', () => {
1113
describe('pathToPosix()', () => {
@@ -29,6 +31,54 @@ describe('util', () => {
2931
})
3032
})
3133

34+
describe('getAutoIncludes()', () => {
35+
const basePath = path.resolve('/project')
36+
const makePm = (name: string): PackageManager => ({
37+
name,
38+
representativeLockfile: undefined,
39+
representativeConfigFile: undefined,
40+
installCommand: () => ({ executable: name, args: ['install'], unsafeDisplayCommand: `${name} install` }),
41+
execCommand: (args: string[]) => ({ executable: name, args, unsafeDisplayCommand: `${name} ${args.join(' ')}` }),
42+
lookupWorkspace: () => Promise.resolve(undefined),
43+
detector: () => ({}) as any,
44+
})
45+
46+
it('should return patches/*.patch for pnpm', () => {
47+
const result = getAutoIncludes(basePath, makePm('pnpm'), [])
48+
expect(result).toEqual(['patches/*.patch'])
49+
})
50+
51+
it('should return empty for npm', () => {
52+
const result = getAutoIncludes(basePath, makePm('npm'), [])
53+
expect(result).toEqual([])
54+
})
55+
56+
it('should return empty for yarn', () => {
57+
const result = getAutoIncludes(basePath, makePm('yarn'), [])
58+
expect(result).toEqual([])
59+
})
60+
61+
it('should skip when user already includes patches/*.patch', () => {
62+
const result = getAutoIncludes(basePath, makePm('pnpm'), ['patches/*.patch'])
63+
expect(result).toEqual([])
64+
})
65+
66+
it('should skip when user already includes a patches/ subpath', () => {
67+
const result = getAutoIncludes(basePath, makePm('pnpm'), ['patches/some-patch.patch'])
68+
expect(result).toEqual([])
69+
})
70+
71+
it('should skip when user includes ./patches/**', () => {
72+
const result = getAutoIncludes(basePath, makePm('pnpm'), ['./patches/**'])
73+
expect(result).toEqual([])
74+
})
75+
76+
it('should skip when user includes absolute patches path', () => {
77+
const result = getAutoIncludes(basePath, makePm('pnpm'), [path.join(basePath, 'patches/**')])
78+
expect(result).toEqual([])
79+
})
80+
})
81+
3282
describe('getPlaywrightVersionFromPackage()', () => {
3383
it('should throw error when playwright package is not found', async () => {
3484
// Use a directory that doesn't have playwright installed

packages/cli/src/services/util.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { PlaywrightConfig } from './playwright-config'
1212
import { Session } from '../constructs/project'
1313
import semver from 'semver'
1414
import { existsSync } from 'fs'
15-
import { detectNearestPackageJson } from './check-parser/package-files/package-manager'
15+
import { detectNearestPackageJson, PackageManager } from './check-parser/package-files/package-manager'
1616

1717
export interface GitInformation {
1818
commitId: string
@@ -143,6 +143,25 @@ export function normalizeVersion (v?: string | undefined): string | undefined {
143143
return cleaned && semver.valid(cleaned) ? cleaned : undefined
144144
}
145145

146+
export function getAutoIncludes (
147+
basePath: string,
148+
packageManager: PackageManager,
149+
existingIncludes: string[],
150+
): string[] {
151+
const autoIncludes: string[] = []
152+
153+
if (packageManager.name === 'pnpm') {
154+
const patchesPattern = 'patches/*.patch'
155+
const patchesDir = path.join(basePath, 'patches')
156+
const alreadyIncluded = existingIncludes.some(p => path.resolve(basePath, p).startsWith(patchesDir))
157+
if (!alreadyIncluded) {
158+
autoIncludes.push(patchesPattern)
159+
}
160+
}
161+
162+
return autoIncludes
163+
}
164+
146165
export async function bundlePlayWrightProject (
147166
playwrightConfig: string,
148167
include: string[],
@@ -195,10 +214,13 @@ export async function bundlePlayWrightProject (
195214
}
196215
}
197216

217+
const autoIncludes = getAutoIncludes(Session.basePath!, Session.packageManager, include)
218+
const effectiveIncludes = [...include, ...autoIncludes]
219+
198220
const includedFiles = await findFilesWithPattern(
199221
// FIXME: Shouldn't the pattern be relative to the Playwright check?
200222
Session.basePath!,
201-
include,
223+
effectiveIncludes,
202224
ignoredFiles,
203225
)
204226

0 commit comments

Comments
 (0)