Refactor component props and enhance type safety #87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Updated MotionEntity to use tuple types for position, rotation, and scale. - Refactored Application, Entity, and various component files to extend PublicProps for better type safety. - Removed deprecated warnOnce utility and improved error handling in components. - Cleaned up commented-out code and improved documentation for clarity. - Enhanced color handling in useMaterial and useColors hooks to support CSS color formats.
commit: |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors component props to improve type safety and code readability by leveraging tuple types, robust prop validation, and dynamic extraction of public properties from PlayCanvas classes.
- Removed the legacy warn-once utility in favor of a consolidated validation module
- Enhanced type definitions and prop validation across components and hooks
- Updated color handling and documentation to ensure consistency with PlayCanvas
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/lib/src/utils/warn-once.ts | Removed legacy warning utility in favor of the new validation module |
| packages/lib/src/utils/validation.ts | Added validation utilities and schema-based prop validation functions |
| packages/lib/src/utils/types-utils.ts | Introduced PublicProps type for filtering out internal members |
| packages/lib/src/utils/color.ts | Enhanced color prop validation and introduced dynamic color property extraction |
| packages/lib/src/hooks/use-material.tsx | Refactored to dynamically retrieve color properties with proper type checking |
| packages/lib/src/components/Anim.tsx | Updated prop interface with a mistake in component type import |
| packages/lib/src/Entity.tsx | Improved prop validation with a schema, with a default rotation inconsistency addressed |
| packages/lib/src/Application.tsx | Refactored Application component to leverage updated type definitions |
| packages/docs/src/components/MotionEntity.tsx | Updated prop types to tuples for improved type safety |
Comments suppressed due to low confidence (1)
packages/lib/src/Entity.tsx:119
- The default rotation value here is inconsistent with the schema, which expects a quaternion ([0, 0, 0, 1]). Update the default to [0, 0, 0, 1] to ensure correct behavior.
rotation = [0, 0, 0],
- Updated AnimProps to extend PublicProps for AnimComponent, enhancing type safety. - Removed commented-out validateAndSanitize function from validation.ts for cleaner code.
- Updated JSDoc comments for Anim, Camera, Collision, GSplat, Light, Render, Script, Sprite, useApp, useMaterial, and useParent components to include parameter descriptions, return types, and usage examples. - Improved clarity and consistency in documentation to aid developers in understanding component usage.
marklundin
added
documentation
- Updated validation utilities to use a new `createComponentDefinition` function, enhancing type safety and clarity in component prop validation. - Refactored multiple components (Application, Entity, Anim, Camera, Collision, GSplat, Light, Render, RigidBody, Script, Sprite) to utilize the new validation structure. - Improved documentation in components to include parameter descriptions and usage examples. - Enhanced error handling and warnings for invalid props, guiding developers towards correct usage.
…or improved mock application handling - Downgraded React and React-DOM to version 18.3.1 in package-lock.json for compatibility. - Refactored multiple components (Application, Anim, Camera, Collision, GSplat, Light, Render, RigidBody, Sprite) to utilize a new `getNullApplication` and `getStaticNullApplication` for mock application instances. - Enhanced validation and component definitions to improve type safety and error handling. - Updated color utility functions to support CSS color formats and improved validation logic.
Contributor
There was a problem hiding this comment.
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/lib/src/hooks/use-script.tsx:74
- The arrow function 'toLowerCamelCase' is defined after its usage in the 'useScript' hook. Since arrow functions are not hoisted, consider moving its declaration above the hook to ensure it is available when used.
const toLowerCamelCase = (str: string) : string => str[0].toLowerCase() + str.substring(1);
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type Improvements
This pull request includes various changes across multiple files to improve type safety, enhance code readability, and update component interfaces. The most important changes involve updating prop types, adding validation, and enhancing documentation.
Types Safety
The type system has been drastically improved and now correctly surfaces the underlying types from the engine. Rather than statically re-declaring engine types, they are dynamically expose from the underlying PlayCanvas engine which is much more flexible and ensures the types always match the engine, regardless of the version installed
Runtime validation
Component types are now validated at runtime, providing helpful feedback for invalid props and falling back to sensible defaults to prevent crashes.
Unknown props
Invalid props
This should drastically improve the developer experience for dynamic prop validation. Warnings are only generated in under
NODE_ENV === 'development'.Changes below
Type Safety and Prop Updates:
packages/docs/src/components/MotionEntity.tsx: Updatedposition,rotation, andscaleprop types to use tuples for better type safety.packages/lib/src/Application.tsx: ReplacedPropsWithChildrenwith explicitchildrenprop and updatedGraphicsOptionstype to usePublicProps<GraphicsDevice>. [1] [2] [3]packages/lib/src/Entity.tsx: Updatedposition,rotation, andscaleprop types to use tuples and added prop validation usingvalidateAndSanitizeProps.Code Refactoring:
packages/lib/src/components/Anim.tsx,Camera.tsx,Collision.tsx,GSplat.tsx,Light.tsx,Render.tsx,RigidBody.tsx,Sprite.tsx: Updated component prop interfaces to usePublicPropsfor better type inference and consistency. [1] [2] [3] [4] [5] [6] [7] [8]Validation and Documentation:
packages/lib/src/Entity.tsx: Added detailed JSDoc comments toEntitycomponent and implemented prop validation schema.packages/lib/src/utils/color.ts: Added regex for hex color validation and integratedvalidateAndSanitizefor color prop validation.Other Improvements:
packages/lib/src/hooks/use-component.tsx: ExportedComponentPropstype for better reusability.packages/lib/src/hooks/use-material.tsx: Improved color prop handling by dynamically generating color property names.These changes collectively enhance the robustness and maintainability of the codebase by leveraging TypeScript's type system and adding necessary validations and documentation.