Skip to content

Conversation

@nvs119
Copy link
Contributor

@nvs119 nvs119 commented Jun 23, 2025

Description

Gearing up to have a separate Compass-Web/API-backed storage implementation that extends this new interface. Changed around some types to keep it still working for existing Compass implementation. Tested on Compass locally, seems to all be working. Waiting for CI tests to continue passing

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@nvs119 nvs119 requested a review from a team as a code owner June 23, 2025 19:53
@nvs119 nvs119 requested a review from Anemy June 23, 2025 19:56
Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

One small request to make the generics stricter, otherwise looks good


export abstract class CompassQueryStorage<
TSchema extends z.Schema,
TData = z.output<TSchema>
Copy link
Collaborator

@gribnoysup gribnoysup Jun 24, 2025

Choose a reason for hiding this comment

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

I think you should either restrict the type here or not make TData a generic at all (and I kinda thing maybe the latter even if it's a bit more verbose), schema and data are connected, so leaving the option to provide data that is not matching the schema for implementations is something we should avoid

Suggested change
TData = z.output<TSchema>
TData extends z.output<TSchema> = z.output<TSchema>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, changed thank you!

@nvs119 nvs119 merged commit 1b53f53 into main Jun 25, 2025
97 of 104 checks passed
@nvs119 nvs119 deleted the COMPASS-9480 branch June 25, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants