Skip to content

Conversation

@wrn14897
Copy link
Member

@wrn14897 wrn14897 commented Dec 30, 2025

Instead of using type assertions to narrow a type, it's better to rely on type guards, which help avoid potential runtime errors caused by unsafe type assertions.

Currently the rule is enforced in common-utils pkg

Dup of #679

@changeset-bot
Copy link

changeset-bot bot commented Dec 30, 2025

🦋 Changeset detected

Latest commit: 2e14eb2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/common-utils Patch
@hyperdx/api Patch
@hyperdx/app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 30, 2025

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

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Dec 30, 2025 2:16am

@claude
Copy link

claude bot commented Dec 30, 2025

PR Review

No critical issues found.

This PR successfully enables the @typescript-eslint/no-unsafe-type-assertion rule and improves type safety across the codebase. The changes are well-structured:

Positive aspects:

  • ✅ Proper type guards added in queryParser.ts (lines 33-53) - clean and correct
  • ✅ Appropriate eslint-disable comments with explanations for necessary type assertions
  • ✅ Fixed unsafe type assertion in filterColumnMetaByType with proper null check (clickhouse/index.ts:799-800)
  • ✅ Improved type safety in metadata.ts with proper type narrowing and null checks
  • ✅ Correctly disabled rule for test files where mocking requires type assertions

Minor suggestions (non-blocking):

  • 💡 metadata.ts:367 - Double filter is redundant (filter on line 367 after already filtering on 364)
  • 💡 Consider documenting the known type issues mentioned in HDX-1548 comment (metadata.ts:770)

Overall: Clean refactor that enhances type safety without introducing bugs. Good incremental improvement to code quality.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2025

E2E Test Results

All tests passed • 52 passed • 5 skipped • 703s

Status Count
✅ Passed 52
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 5

Tests ran across 4 shards in parallel.

View full report →

'@typescript-eslint/no-empty-object-type': 'warn',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-floating-promises': 'error',
'@typescript-eslint/no-unsafe-type-assertion': 'error',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type assertion will error in common-utils pkg going forward

@wrn14897 wrn14897 force-pushed the warren/introduce-ts-no-unsafe-type-assertion-eslint-rule branch from e33b751 to 54f5752 Compare December 30, 2025 02:11
@wrn14897 wrn14897 changed the title chore(eslint): enable @typescript-eslint/no-unsafe-type-assertion rule chore(eslint): add @typescript-eslint/no-unsafe-type-assertion rule Dec 30, 2025
@kodiakhq kodiakhq bot merged commit 103c63c into main Dec 30, 2025
14 checks passed
@kodiakhq kodiakhq bot deleted the warren/introduce-ts-no-unsafe-type-assertion-eslint-rule branch December 30, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants