Conversation
|
72636c
left a comment
72636c
left a comment
There was a problem hiding this comment.
Seems reasonable enough, though agree with Sam that magic is not ideal 🥲
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/seek-oss/skuba.git", | ||
| "directory": "packages/pnpm-plugin" |
There was a problem hiding this comment.
Oh TIL directory, we should probably set it for our other packages
packages/pnpm-plugin/pnpmfile.cjs
Outdated
| // @ts-expect-error Property isn't in PnpmSettings for some reason | ||
| config.publicHoistPattern ??= []; | ||
| // @ts-expect-error Property isn't in PnpmSettings for some reason | ||
| config.publicHoistPattern.push( |
There was a problem hiding this comment.
Should our patterns be prepended so consumers can define overrides with !?
packages/pnpm-plugin/pnpmfile.cjs
Outdated
| // @ts-expect-error Property isn't in PnpmSettings for some reason | ||
| config.packageManagerStrictVersion = true; |
There was a problem hiding this comment.
Similar question on whether stuff like this should be ??= to allow consumer to override 🤔
| "peerDependencies": { | ||
| "pnpm": "<10.13.0" | ||
| } |
There was a problem hiding this comment.
I don't know if this will do the same thing as https://github.com/seek-oss/sku/blob/831bf9151023c9b5ec7b13288f3f40626deacf90/packages/utils/src/packageManager/packageManager.ts#L89-L94
There was a problem hiding this comment.
I would assume not, but worth testing alongside https://docs.npmjs.com/cli/v11/configuring-npm/package-json#engines and https://docs.npmjs.com/cli/v11/configuring-npm/package-json#devengines. Also depends on whether we want to log a warning or fail the install if an older pnpm version is detected.
| config.publicHoistPattern ??= [ | ||
| '@types*', | ||
| 'eslint', | ||
| 'eslint-config-skuba', | ||
| 'prettier', | ||
| 'esbuild', | ||
| 'jest', | ||
| 'tsconfig-seek', | ||
| 'typescript', | ||
| ]; |
There was a problem hiding this comment.
To support merging, the array configs could be like
| config.publicHoistPattern ??= [ | |
| '@types*', | |
| 'eslint', | |
| 'eslint-config-skuba', | |
| 'prettier', | |
| 'esbuild', | |
| 'jest', | |
| 'tsconfig-seek', | |
| 'typescript', | |
| ]; | |
| (config.publicHoistPattern ??= []).unshift( | |
| '@types*', | |
| 'eslint', | |
| 'eslint-config-skuba', | |
| 'prettier', | |
| 'esbuild', | |
| 'jest', | |
| 'tsconfig-seek', | |
| 'typescript', | |
| ); |
Or, if the above is too spooky to look at:
| config.publicHoistPattern ??= [ | |
| '@types*', | |
| 'eslint', | |
| 'eslint-config-skuba', | |
| 'prettier', | |
| 'esbuild', | |
| 'jest', | |
| 'tsconfig-seek', | |
| 'typescript', | |
| ]; | |
| config.publicHoistPattern = [ | |
| '@types*', | |
| 'eslint', | |
| 'eslint-config-skuba', | |
| 'prettier', | |
| 'esbuild', | |
| 'jest', | |
| 'tsconfig-seek', | |
| 'typescript', | |
| ...(config.publicHoistPattern ?? []), | |
| ]; |
There was a problem hiding this comment.
:\ I'm still not a huge fan of this being done in the background where it's less obvious to consumers
There was a problem hiding this comment.
This seems to imply that plugins don't work properly with pnpm fetch and pnpm install --offline 🤔
4 participants
No description provided.