fix: recs remove component on empty values #427
Conversation
WalkthroughThis pull request introduces several adjustments across the codebase. A new changeset entry for the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Client Code
participant SE as setEntities
participant CH as Component Handler
participant Log as Logger
Caller->>SE: Call setEntities(entity)
SE->>CH: Check for existence of recsComponent
alt Component exists
SE->>SE: Verify rawValue
alt rawValue is empty
SE->>Log: Log "Component removal" warning
SE->>CH: Invoke removeComponent(rawValue)
else rawValue valid
SE->>Log: Log rawValue and convertedValue details
alt Conversion fails
SE->>Log: Log error message
else Conversion successful
SE->>CH: Invoke setComponent(convertedValue)
end
end
else
SE->>Log: Log warning indicating missing component
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
.changeset/blue-points-drop.md (1)
1-6: Clear Changelog Entry for Empty Component Removal
The changeset entry clearly documents the patch for@dojoengine/stateand details the fix related to removing components with empty values. For improved clarity and future maintainability, consider adding a brief note or reference (e.g., an issue number or test coverage details) that explains the context of this change.packages/state/src/recs/index.ts (1)
417-426: Fixed grammar in console log messages.The log messages have been corrected from "Did not found" to "Did not find".
These are minor improvements to log message readability.
packages/sdk/src/react/hooks/state.ts (2)
33-33: Removed trailing comma in type declaration.This is a minor syntactic correction that improves code style consistency.
60-60: Removed trailing comma in type declaration.This is a minor syntactic correction that improves code style consistency.
packages/sdk/src/types.ts (4)
251-251: Removed trailing comma in type declaration for ToriiResponse.This is a minor syntactic correction that improves code style consistency across type definitions.
258-258: Removed trailing comma in type declaration for SubscribeResponse.This is a minor syntactic correction that improves code style consistency across type definitions.
526-526: Removed trailing comma in type declaration for SubscribeParams.This is a minor syntactic correction that improves code style consistency across interface type parameters.
538-538: Removed trailing comma in type declaration for GetParams.This is a minor syntactic correction that improves code style consistency across interface type parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/blue-points-drop.md(1 hunks)package.json(1 hunks)packages/sdk/src/react/hooks/hooks.ts(1 hunks)packages/sdk/src/react/hooks/index.ts(1 hunks)packages/sdk/src/react/hooks/state.ts(2 hunks)packages/sdk/src/react/provider.tsx(1 hunks)packages/sdk/src/types.ts(3 hunks)packages/state/src/recs/index.ts(3 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/sdk/src/react/hooks/index.ts (2)
packages/sdk/src/types.ts (1)
SchemaType(51-71)examples/example-vite-token-balance/src/typescript/models.gen.ts (1)
SchemaType(68-80)
packages/state/src/recs/index.ts (1)
packages/state/src/utils/index.ts (1)
convertValues(3-62)
packages/sdk/src/react/hooks/state.ts (2)
packages/sdk/src/types.ts (1)
SchemaType(51-71)examples/example-vite-token-balance/src/typescript/models.gen.ts (1)
SchemaType(68-80)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (12)
package.json (1)
16-16: New Dev Dependency Script Added
The addition of"dev:deps": "turbo watch build:deps --concurrency 20"is a useful enhancement that enables dedicated watching of dependency builds with a specified concurrency. Ensure that this script aligns with your development environment’s performance expectations and does not conflict with other dev scripts.packages/sdk/src/react/hooks/index.ts (1)
20-22: Remove Trailing Comma in Generic Type Parameter
The removal of the trailing comma from the generic type declaration (i.e. changingSchema extends SchemaType,toSchema extends SchemaType) is a clean formatting improvement. It enhances consistency across similar declarations in the codebase without affecting functionality.🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/react/provider.tsx (1)
16-19: Formatting Improvement in DojoContextType Declaration
The consolidation of the type parameters (i.e. the removal of the trailing comma afterClient extends (...args: any) => any) in theDojoContextTypeinterface enhances readability and consistency with other parts of the codebase. This is a purely cosmetic change with no impact on functionality.🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 17-17: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/react/hooks/hooks.ts (1)
15-18: Remove Trailing Comma in Generic Type Parameter
The update removes the trailing comma from the generic type parameter declaration in thecreateSubscriptionHookfunction (Historical extends boolean = false). This change aligns with similar formatting improvements elsewhere in the codebase, contributing to cleaner and more consistent type definitions.packages/state/src/recs/index.ts (8)
2-6: Improved type safety with explicit type imports.Using the
typekeyword for imports ensures these are treated as type-only imports, which can help with tree-shaking and prevents accidental runtime usage of type information.
9-9: Added removeComponent import to support empty value handling.This import is necessary to support the new functionality for removing components when they contain empty values.
13-20: Improved import organization with the type keyword.Using a dedicated
import typestatement for these imports improves code organization and makes it clear that these are type-only imports.
434-440: Improved error handling with early returns.The code now checks for the existence of
recsComponentfirst and returns early if not found, improving readability and maintainability.
442-453: Implemented component removal for empty values.This is the core implementation of the PR fix. The code now checks if
rawValueis an empty object (no keys) and removes the component in that case, preventing empty components from being stored.This change is the main feature mentioned in the PR title "fix: recs remove component on empty values".
455-470: Enhanced logging for better debugging.Improved the logging of raw and converted values, providing more context by including the component name and entity key in the log messages.
472-476: Added validation for convertedValue.Added an explicit check to ensure convertedValue is valid before continuing with component operations.
478-492: Reorganized component update logic.The code flow has been restructured to be more logical:
- First, check if the component exists
- If so, update it and continue
- Otherwise, set the component
This improves readability and better separates the update vs. set operations.
MartianGreed
force-pushed
the
fix/recs-remove-component
branch
from
667a753 to
6becde8
Compare
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
Release Notes:
This update improves the overall display and reliability of the system by ensuring that only relevant components are shown. Users will notice a cleaner interface as components without data are now automatically removed.
Bug Fixes
New Features