fix: remove temporary ESLint exclusions for re-export violations #3839
Conversation
- Move re-exports from DropdownMenu.tsx to index.ts file - Move PostgreSQL parser re-export from parser.ts to parser/index.ts - Remove temporary ESLint exclusions from base.js configuration - Maintain same public API while following re-export guidelines Resolves liam-hq#2349
sushilk123
requested review from
MH4GF,
NoritakaIkeda,
junkisai and
sasamuku
and removed request for
a team
|
Finished running flow.
|
|
Someone is attempting to deploy a commit to the Liam Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughRemoved per-file ESLint overrides for DropdownMenu and parser files; moved DropdownMenu re-exports into its Changes
Sequence Diagram(s)(omitted — changes are export/refactor-only and do not alter runtime control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Potential attention points:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Check changeset necessityStatus: NOT REQUIRED Reason:
Changeset (copy & paste):# No changeset required — internal refactor with no user-facing changes in @liam-hq/schema or @liam-hq/ui. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Thanks! The diff is LGTM, but the formatter failed, so please check it.
https://github.com/liam-hq/liam/actions/runs/18695870345/job/53336274540?pr=3839
- Reorder exports to satisfy Biome linter requirements - Move named exports before wildcard export for proper organization
sushilk123
force-pushed
the
fix/remove-eslint-re-export-exclusions
branch
from
eb53d8c to
71b4236
Compare
- Replace 'export {} from' syntax with separate import and export statements
- Maintains same public API while satisfying ESLint no-restricted-syntax rule
- Keeps parser.ts file to maintain package.json export compatibility
|
@sushilk123 It seems lint is still failing. Try running |
- Split type imports into separate import statement - Split type exports into separate export statement - Follows Biome's preferred import/export organization
…nstanceManager - Add proper type guards for PgParseResult to handle error and success cases - Replace unsafe member access with type-safe checks using isParseError/isParseSuccess - Isolate type assertions to dedicated helper functions - Add fallback handling for edge cases where neither error nor parse_tree exist Fixes unsafe member access violations on parseResult.error and parseResult.parse_tree
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
Files:
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
frontend/**/*.{ts,tsx}: Use named exports only (no default exports)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Follow existing import patterns and tsconfig path aliases
Use consts instead of function declarations for simple utilities (e.g., const toggle = () => {})
Use runtime type validation with valibot for external data validation
Use early returns for readability
Files:
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts
🧬 Code graph analysis (1)
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts (1)
frontend/internal-packages/pglite-server/src/types.ts (1)
SqlResult(1-11)
🔇 Additional comments (1)
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts (1)
81-93: Good refactor for readability, but depends on fixing the type guards.The explicit branching with early returns improves code clarity and follows the coding guidelines. The fallback case on line 93 is good defensive programming.
However, the effectiveness of this refactor depends on fixing the type guard implementations (lines 65-75) to ensure type-safe narrowing.
| private isParseError( | ||
| result: PgParseResult, | ||
| ): result is { error: { message: string } } { | ||
| return (result as any).error !== null | ||
| } | ||
|
|
||
| private isParseSuccess( | ||
| result: PgParseResult, | ||
| ): result is { parse_tree: { stmts: any[] } } { | ||
| return (result as any).parse_tree?.stmts != null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify PgParseResult type structure to ensure correct type guards.
The type guards assume a specific discriminated union structure for PgParseResult. Please verify the actual type definition to ensure the guards correctly match the type's structure.
Run the following script to examine the PgParseResult type definition:
🏁 Script executed:
#!/bin/bash
# Description: Find and display the PgParseResult type definition from the parser module
# Search for PgParseResult type definition
ast-grep --pattern 'type PgParseResult = $$$'
# Also search for interface definitions
ast-grep --pattern 'interface PgParseResult {
$$$
}'
# Search for export statements
rg -nP --type=ts 'export.*PgParseResult' -A 5Length of output: 2518
Fix type guard return types to properly narrow PgParseResult.
The logic in both guards is correct, but the return type annotations are incomplete. They only reflect the discriminating field (error or parse_tree) while omitting other required fields, breaking type safety for callers. The return types should preserve the full structure with narrowed error:
isParseError: return type should beresult is PgParseResult & { error: NonNullable<PgParseResult['error']> }isParseSuccess: return type should beresult is PgParseResult & { error: null }
This ensures callers can still access stderr_buffer and other fields after the guard.
🤖 Prompt for AI Agents
In frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts around
lines 65 to 75, the two type guard return annotations are too narrow and drop
other PgParseResult fields; update them so isParseError returns result is
PgParseResult & { error: NonNullable<PgParseResult['error']> } and
isParseSuccess returns result is PgParseResult & { error: null }, keeping the
existing runtime checks but changing only the return type signatures so callers
retain access to fields like stderr_buffer and other PgParseResult properties
after narrowing.
| private isParseError( | ||
| result: PgParseResult, | ||
| ): result is { error: { message: string } } { | ||
| return (result as any).error !== null | ||
| } |
There was a problem hiding this comment.
Fix type guard implementation to avoid unsafe type assertions.
The type guard uses as any, which defeats TypeScript's type safety and can lead to runtime errors. Additionally, checking error !== null without first verifying the property exists could throw if error is undefined.
Apply this diff to use proper type narrowing:
private isParseError(
result: PgParseResult,
): result is { error: { message: string } } {
- return (result as any).error !== null
+ return 'error' in result && result.error != null
}Alternatively, if the property is always present but may be null/undefined:
private isParseError(
result: PgParseResult,
): result is { error: { message: string } } {
- return (result as any).error !== null
+ return (result as { error?: { message: string } | null }).error != null
}🤖 Prompt for AI Agents
In frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts around
lines 65 to 69, the type guard uses an unsafe cast to any and only checks error
!== null which can throw if the property is missing; change the implementation
to first check that the 'error' property exists (using the 'in' operator or
typeof) and then test for non-null/undefined (error != null), e.g. return
'error' in result && result.error != null, so you avoid any type assertions and
properly narrow the type.
| private isParseSuccess( | ||
| result: PgParseResult, | ||
| ): result is { parse_tree: { stmts: any[] } } { | ||
| return (result as any).parse_tree?.stmts != null | ||
| } |
There was a problem hiding this comment.
Remove unsafe type assertion from type guard.
The type guard uses as any, which bypasses TypeScript's type checking. While the optional chaining provides some safety, proper type narrowing is more robust.
Apply this diff to use the in operator for type-safe narrowing:
private isParseSuccess(
result: PgParseResult,
): result is { parse_tree: { stmts: any[] } } {
- return (result as any).parse_tree?.stmts != null
+ return 'parse_tree' in result && Array.isArray(result.parse_tree?.stmts)
}🤖 Prompt for AI Agents
In frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts around
lines 71 to 75, remove the unsafe "as any" assertion in the type guard and
replace it with proper property-in checks: first test whether "parse_tree" is in
result and that result.parse_tree is an object, then test whether "stmts" is in
result.parse_tree (and is an array if desired) using the "in" operator and
runtime checks; update the return type accordingly so the guard returns true
only when those properties exist and are correctly shaped, avoiding any use of
"as any".
2 participants
Summary
Removes temporary ESLint exclusions for re-export violations by refactoring the problematic files to follow the established re-export guidelines.
Changes
DropdownMenu.tsxtoindex.tsparser.tstoparser/index.tsbase.jsTesting
Resolves
Fixes #2349
Summary by CodeRabbit
Refactor
Chores