liam icon indicating copy to clipboard operation
liam copied to clipboard

fix: remove temporary ESLint exclusions for re-export violations

Open sushilk123 opened this issue 2 months ago • 6 comments

Summary

Removes temporary ESLint exclusions for re-export violations by refactoring the problematic files to follow the established re-export guidelines.

Changes

  • DropdownMenu: Moved Radix UI re-exports from DropdownMenu.tsx to index.ts
  • Parser: Moved PostgreSQL parser re-export from parser.ts to parser/index.ts
  • ESLint Config: Removed temporary exclusions from base.js

Testing

  • All re-export violations resolved
  • Public API maintained unchanged
  • ESLint rules now apply consistently across codebase

Resolves

Fixes #2349

Summary by CodeRabbit

  • Refactor

    • Consolidated parser exports into a single public API and surfaced PostgreSQL parsing utilities and related types from the central parser entry.
    • Adjusted DropdownMenu export surface so primitives (item indicator and radio item) are exposed from the public component entry.
    • Added internal parsing guards to improve parse result handling and error branching.
  • Chores

    • Removed a local lint override so affected files now follow global linting rules.

sushilk123 avatar Oct 21 '25 19:10 sushilk123

Finished running flow.

Step Status Updated(UTC)
1 Oct 21, 2025 7:51pm
2 Oct 21, 2025 7:53pm
3 Oct 21, 2025 7:53pm

giselles-ai[bot] avatar Oct 21 '25 19:10 giselles-ai[bot]

Someone is attempting to deploy a commit to the Liam Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Oct 21 '25 19:10 vercel[bot]

Walkthrough

Removed per-file ESLint overrides for DropdownMenu and parser files; moved DropdownMenu re-exports into its index.ts; consolidated parser public exports through parser/index.ts (pgParse and PgParseResult re-exported). No runtime logic changes.

Changes

Cohort / File(s) Summary
ESLint config
frontend/internal-packages/configs/eslint/base.js
Removed the per-file override/exclusion for the DropdownMenu and parser files so they now follow the global no-restricted-syntax rule.
Parser public API
frontend/packages/schema/src/parser.ts, frontend/packages/schema/src/parser/index.ts
Centralized parser exports to ./parser/index.ts: re-exported pgParse (alias of PostgreSQL parse) and PgParseResult type; other parser exports (detectFormat, ProcessError, parse, setPrismWasmUrl, supportedFormatSchema) now re-export from the centralized index.
DropdownMenu exports
frontend/packages/ui/src/components/DropdownMenu/DropdownMenu.tsx, frontend/packages/ui/src/components/DropdownMenu/index.ts
Removed re-exports from DropdownMenu.tsx; added named re-exports in index.ts (ItemIndicatorDropdownMenuItemIndicator, RadioItemDropdownMenuPrimitiveRadioItem). Implementation unchanged.
PGlite parse handling
frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts
Added private type-guard helpers isParseError and isParseSuccess; refactored executeSql to use these guards and added a fallback parse-error return when neither guard matches.

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:

  • frontend/internal-packages/pglite-server/src/PGliteInstanceManager.ts — verify the type-guard logic matches all PgParseResult shapes.
  • Parser re-exports (frontend/packages/schema/src/parser.ts and .../index.ts) — confirm no accidental API surface changes and correct type-only vs runtime exports.
  • ESLint config change — ensure lint now passes for the moved re-exports.

Possibly related PRs

  • liam-hq/liam#3716 — related to PostgreSQL parser exports and PgParseResult/pgParse changes.
  • liam-hq/liam#3114 — centralizes PostgreSQL parser usage and aligns with parser re-export refactor.
  • liam-hq/liam#2934 — modifies PGliteInstanceManager parsing/execution behavior similar to the added type-guards.

Suggested labels

Review effort 3/5

Suggested reviewers

  • junkisai
  • NoritakaIkeda
  • sasamuku
  • hoshinotsuyoshi

Poem

🐇 I hopped through modules, neat and spry,
moved exports where the indexes lie.
ESLint nods, no special case,
tidy trees in the codebase space,
a rabbit smiles — tidy bytes go by.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The changes to PGliteInstanceManager.ts introduce private type-guard helper methods (isParseError and isParseSuccess) and refactor the executeSql method to use these guards. While these changes are related to the PgParseResult type being refactored, they go beyond the stated scope of issue #2349, which explicitly targets only moving re-exports to index.ts files and removing ESLint exclusions. The addition of defensive type guards represents an implementation change not required by the issue objectives. Clarify whether the PGliteInstanceManager.ts changes are necessary adaptations for the refactored types or if they should be extracted into a separate follow-up PR. If these changes are necessary, update the PR description to document why they were required. If not, consider reverting them to keep this PR focused on the explicit goal of moving re-exports and removing ESLint exclusions.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "fix: remove temporary ESLint exclusions for re-export violations" is clear, concise, and directly related to the primary objective of the changeset. It accurately summarizes the main intent of the changes: removing temporary ESLint exclusions by refactoring files to comply with re-export guidelines. The title uses a conventional prefix and conveys the essential change without unnecessary details.
Linked Issues Check ✅ Passed The code changes directly address all primary requirements from linked issue #2349. The DropdownMenu re-exports (ItemIndicator and RadioItem) have been successfully moved from DropdownMenu.tsx to index.ts, the PostgreSQL parser re-export (pgParse) has been moved from parser.ts to parser/index.ts with appropriate type exports, and the temporary ESLint exclusions for both files have been removed from the base configuration. These changes follow the established re-export guidelines by centralizing re-exports in index.ts files as required.
Description Check ✅ Passed The PR description provides substantive content covering the summary, specific changes made to each file, testing approach, and issue resolution link. While it doesn't strictly follow the template structure (which specifies "Issue: resolve:" and "Why is this change needed?" sections), the author has effectively communicated what was changed (DropdownMenu, Parser, ESLint Config), why (to remove temporary exclusions), and how it was validated. The description includes sufficient information for reviewers to understand the scope and rationale.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 21 '25 19:10 coderabbitai[bot]

Check changeset necessity

Status: NOT REQUIRED

Reason:

  • Affects target packages @liam-hq/schema and @liam-hq/ui, but only reorganizes re-exports (moved PostgreSQL parser exports and Radix UI re-exports to index files) with the public API explicitly maintained unchanged.
  • No user-facing changes: no new features, bug fixes, API or behavioral changes; added displayName is devtools-only.
  • ESLint config changes are development tooling in an internal configs package and do not impact published packages.
  • Purpose is to remove temporary lint exclusions and align with re-export guidelines; this is internal refactoring.

Changeset (copy & paste):

# No changeset required — internal refactor with no user-facing changes in @liam-hq/schema or @liam-hq/ui.

giselles-ai[bot] avatar Oct 21 '25 19:10 giselles-ai[bot]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
liam-app Ready Ready Preview Comment Oct 22, 2025 2:25am
liam-erd-sample Canceled Canceled Oct 22, 2025 2:25am
liam-storybook Ready Ready Preview Comment Oct 22, 2025 2:25am

vercel[bot] avatar Oct 22 '25 02:10 vercel[bot]

@sushilk123 It seems lint is still failing. Try running pnpm fmt. Also, if you click Re-request review, I'll review it right away!

MH4GF avatar Oct 24 '25 06:10 MH4GF