-
Notifications
You must be signed in to change notification settings - Fork 15
fix: lint issues #363
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
base: main
Are you sure you want to change the base?
fix: lint issues #363
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| --- | ||
| name: pnpm-postinstall-deps | ||
| description: Use when user adds package with postinstall script (supabase, sharp), gets "binary not found" errors in CI, or asks about onlyBuiltDependencies. Explains the two-step pattern for managing packages that need build/postinstall scripts in pnpm workspaces with frozen lockfiles. | ||
| --- | ||
|
|
||
| # pnpm Postinstall Dependencies | ||
|
|
||
| Manage packages that need postinstall scripts in pnpm workspaces. | ||
|
|
||
| ## The Problem | ||
|
|
||
| `pnpm install --frozen-lockfile --prefer-offline` in CI restores from cache and skips postinstall scripts. Packages like `supabase` download binaries via postinstall - without it, the binary doesn't exist. | ||
|
|
||
| ## The Pattern | ||
|
|
||
| **Two-step sync required:** | ||
|
|
||
| 1. **pnpm-workspace.yaml** - Declares which packages are allowed to run build scripts | ||
| 2. **Setup action** - Explicitly rebuilds those packages after install | ||
|
|
||
| <critical> | ||
| Both lists MUST stay in sync. If they diverge: | ||
| - Missing in yaml → Package can't run postinstall (security) | ||
| - Missing in action → Binary won't be available in CI | ||
| </critical> | ||
|
|
||
| ## Checklist | ||
|
|
||
| When adding a package that needs postinstall: | ||
|
|
||
| ### 1. Add to pnpm-workspace.yaml | ||
|
|
||
| ```yaml | ||
| onlyBuiltDependencies: | ||
| - supabase | ||
| - sharp | ||
| - your-new-package # Add here | ||
| ``` | ||
|
|
||
| ### 2. Update .github/actions/setup/action.yml | ||
|
|
||
| ```yaml | ||
| - name: Rebuild packages that need postinstall (onlyBuiltDependencies) | ||
| shell: bash | ||
| run: pnpm rebuild supabase sharp your-new-package # Add here | ||
| ``` | ||
|
|
||
| ### 3. Verify | ||
|
|
||
| Check both files have identical package lists. | ||
|
|
||
| ## Why Explicit Package Names? | ||
|
|
||
| `pnpm rebuild --pending` is too implicit - unclear if it respects onlyBuiltDependencies. Explicit names guarantee behavior and make configuration visible. | ||
|
|
||
| ## Common Packages | ||
|
|
||
| - `supabase` - Downloads CLI binary from GitHub releases | ||
| - `sharp` - Builds native image processing library | ||
| - `esbuild` - Downloads platform-specific binary | ||
| - `puppeteer` - Downloads Chromium binary |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,16 +7,17 @@ export default defineConfig({ | |
| root: __dirname, | ||
| cacheDir: '../../node_modules/.vite/pkgs/client', | ||
| plugins: [ | ||
| dts({ | ||
| dts({ | ||
| include: ['src/**/*.ts'], | ||
| outDir: 'dist', | ||
| rollupTypes: false, // Don't bundle for now | ||
| insertTypesEntry: true, | ||
| tsConfigFilePath: resolve(__dirname, 'tsconfig.lib.json'), | ||
| skipDiagnostics: true // Skip TypeScript diagnostics to avoid vite-plugin-dts errors with monorepo project references. | ||
| // The plugin tries to compile all imported files (including from other packages) | ||
| tsconfigPath: resolve(__dirname, 'tsconfig.lib.json'), | ||
| skipDiagnostics: true // Skip TypeScript diagnostics to avoid vite-plugin-dts errors with monorepo project references. | ||
| // The plugin tries to compile all imported files (including from other packages) | ||
| // which breaks rootDir boundaries. Nx runs proper type checking separately. | ||
| }) | ||
| }) as any // Type cast to avoid Vite version mismatch between packages | ||
|
Comment on lines
+15
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The property was renamed from 'tsConfigFilePath' to 'tsconfigPath' and a type cast was added. This might be causing compatibility issues. Verify the correct property name for the version of vite-plugin-dts being used and remove the type cast if possible. Spotted by Graphite Agent (based on CI logs) |
||
|
|
||
| ], | ||
| build: { | ||
| lib: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,7 +178,7 @@ describe('.array() method type constraints', () => { | |
|
|
||
| // ExtractFlowContext should include FlowContext & custom resources | ||
| type FlowCtx = ExtractFlowContext<typeof flow>; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An eslint-disable comment was removed. If this was intentional, ensure the code doesn't trigger the eslint rule that was being disabled. Otherwise, restore the comment. Spotted by Graphite Agent (based on CI logs) |
||
| expectTypeOf<FlowCtx>().toMatchTypeOf<{ | ||
| env: Record<string, string | undefined>; | ||
| shutdownSignal: AbortSignal; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,9 @@ | |
| }, | ||
| { | ||
| "path": "./pkgs/client" | ||
| }, | ||
| { | ||
| "path": "./apps/demo" | ||
| } | ||
| ] | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding 'composite: true' requires additional configuration. Either remove this property or add required settings like 'rootDir' and 'outDir'. Composite projects have stricter requirements that may be causing TypeScript errors.
Spotted by Graphite Agent (based on CI logs)

Is this helpful? React 👍 or 👎 to let us know.