Repository navigation
fix: Fix build and potential Mosaic duckdb-wasm type mismatch - #625
Conversation
📝 WalkthroughWalkthroughA single type-suppression comment change in ChangesType-suppression directive update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mosaic/src/MosaicSlice.ts (1)
161-166: ⚡ Quick winUse
@ts-expect-errorinstead of@ts-ignoreto prevent stale suppressions.
@ts-expect-errorwill fail compilation if the underlying type error is resolved, ensuring suppressions don't become stale.@ts-ignorewill continue to suppress even when no error exists, potentially masking future issues.Additionally, using
eslint-disableto suppressban-ts-commentconflicts with the lint baseline requirement of warnings only. If possible, consider whether a type assertion (e.g.,as duckdb.AsyncDuckDB) would be more appropriate, though this would require verifying compatibility with Mosaic's expected types.♻️ Switch to `@ts-expect-error`
- // eslint-disable-next-line `@typescript-eslint/ban-ts-comment` - // `@ts-ignore` - We might be using a different version of duckdb-wasm than mosaic expects + // `@ts-expect-error` - We might be using a different version of duckdb-wasm than mosaic expects duckDb: dbConnector.getDb(), - // eslint-disable-next-line `@typescript-eslint/ban-ts-comment` - // `@ts-ignore` - same version mismatch + // `@ts-expect-error` - same version mismatch connection: dbConnector.getConnection(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mosaic/src/MosaicSlice.ts` around lines 161 - 166, Replace the two "// `@ts-ignore`" suppressions used for duckDb: dbConnector.getDb() and connection: dbConnector.getConnection() with "// `@ts-expect-error`" and remove the corresponding "eslint-disable-next-line `@typescript-eslint/ban-ts-comment`" lines so the suppression fails fast if the type issue is resolved; alternatively, if you can confirm types, replace the ignores entirely with explicit type assertions (e.g., cast dbConnector.getDb() to the expected duckdb.AsyncDuckDB and dbConnector.getConnection() to the expected connection type) and remove both the ts-comment and eslint-disable lines for cleaner, non-stale typing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/mosaic/src/MosaicSlice.ts`:
- Around line 161-166: Replace the two "// `@ts-ignore`" suppressions used for
duckDb: dbConnector.getDb() and connection: dbConnector.getConnection() with "//
`@ts-expect-error`" and remove the corresponding "eslint-disable-next-line
`@typescript-eslint/ban-ts-comment`" lines so the suppression fails fast if the
type issue is resolved; alternatively, if you can confirm types, replace the
ignores entirely with explicit type assertions (e.g., cast dbConnector.getDb()
to the expected duckdb.AsyncDuckDB and dbConnector.getConnection() to the
expected connection type) and remove both the ts-comment and eslint-disable
lines for cleaner, non-stale typing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3db02390-7a0a-4035-9ef9-011fa4af92c2
📒 Files selected for processing (1)
packages/mosaic/src/MosaicSlice.ts
ilyabo
changed the title
Summary by CodeRabbit