Skip to content

fix: types #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 18, 2024
Merged

fix: types #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/sdk/src/getEntities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export async function getEntities<T extends SchemaType>(
limit: limit,
offset: cursor,
clause,
dont_include_hashed_keys: query.entityIds ? false : true,
};

try {
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/src/getEventMessages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export async function getEventMessages<T extends SchemaType>(
limit: limit,
offset: cursor,
clause,
dont_include_hashed_keys: true,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Update torii.Query type definition to include dont_include_hashed_keys

The property dont_include_hashed_keys: true is being used in several instances of getEventMessages, but it is not defined in the torii.Query type. This inconsistency can lead to type errors and unexpected behaviors across the codebase.

Action Items:

  1. Update Type Definitions:

    • Add dont_include_hashed_keys?: boolean to the torii.Query type to ensure it is recognized across all usages.

  2. Ensure Consistency:

    • Verify that all instances of getEventMessages either include this property or handle its absence appropriately.

  3. Update Documentation:

    • Reflect the addition of dont_include_hashed_keys in the JSDoc comments and any relevant documentation to inform users of its purpose and usage.

  4. Review Usage:

    • Check for any conditional logic that relies on the presence or absence of dont_include_hashed_keys to ensure consistent behavior.

Category:

🔗 Analysis chain

Update documentation and consider backwards compatibility for dont_include_hashed_keys

The addition of dont_include_hashed_keys: true to the toriiQuery object modifies the behavior of the query sent to the Torii client. While this change may improve performance by reducing the amount of data returned, it raises several concerns:

  1. The function's JSDoc comments don't mention this new behavior, which could lead to confusion for users of the SDK.
  2. This change might break existing code that expects hashed keys to be present in the query results.
  3. The type definition for torii.Query may need to be updated to include this new property.

To address these issues, consider the following:

  1. Update the function's JSDoc comments to explain the purpose and impact of dont_include_hashed_keys.
  2. Consider making this behavior optional by adding a parameter to the function, allowing users to choose whether to include hashed keys or not.
  3. Update the torii.Query type definition if necessary.
  4. If this is a breaking change, consider versioning the SDK appropriately and documenting the change in the changelog.

To verify the impact of this change, we can search for other usages of getEventMessages in the codebase:

This will help us understand if other parts of the codebase might be affected by this change.

[/run_scripts]

[/run_scripts]

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of getEventMessages
rg --type typescript 'getEventMessages\(' -A 10

Length of output: 84

Script:

#!/bin/bash
# Search for usages of getEventMessages in TypeScript files using correct file type
rg --type ts 'getEventMessages\(' -A 10

# If the above does not work, search using file extensions directly
rg 'getEventMessages\(' --glob '*.ts' --glob '*.tsx' -A 10

Length of output: 5598

Script:

#!/bin/bash
# Search for the definition of torii.Query type
ast-grep --lang typescript --pattern 'type torii\.Query {$$$}' packages/sdk/src/**/*.ts

# Alternatively, search for interfaces named torii.Query
rg 'interface\s+torii\.Query' --glob '*.ts' 

# Check for occurrences of dont_include_hashed_keys in type definitions
rg 'dont_include_hashed_keys' --glob '*.ts'

Length of output: 1374

};

try {
Expand Down
Loading