Feat support router subpage #935
Conversation
…RenderMain and Render
…List type, separate isObject-parser into 3 parsers
…ge scoped css, add RenderView logic to render, add page-getter, extra useLocale
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a comprehensive migration towards TypeScript in the canvas rendering package. The changes span multiple files and involve adding TypeScript support, updating configuration files, introducing new utility functions, and refactoring existing components. The primary focus is on enhancing type safety, modularizing code, and preparing the codebase for more robust TypeScript integration. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Config as ESLint/Vite Config
participant Canvas as Canvas Rendering
participant Components as Vue Components
Dev->>Config: Add TypeScript support
Config->>Canvas: Configure TypeScript extensions
Canvas->>Components: Migrate to TypeScript
Components->>Canvas: Implement new TypeScript utilities
Canvas-->>Dev: Enhanced type-safe rendering system
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
…pport-router-subpage
…child page's parent view is not updated to latest version
…pport-router-subpage
…ecursive load pathFromRoot[0] page issue
…mpty children, remove builtin CanvasBox placeholder
…efault schema, update page id in location params
…ages, fix unsubscribe topic no callback function issue
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (51)
packages/canvas/render/src/canvas-function/CanvasEmpty.vue (2)
2-2: Consider introducing localized or configurable default text.You're providing a fallback text in Chinese, which is fine if this application is dedicated to that locale. If you plan to expand into multiple locales, you might integrate a localization library or dynamic placeholders in the future.
15-19: Use a typed approach forplaceholderText.Currently,
placeholderTextis declared withString; however, you might benefit from a typed definition using TypeScript’sdefineProps<T>()pattern. This way, you ensure stricter type checking and enable optional usage in the template.Consider applying this diff to switch to a typed approach:
-<script setup> -const props = defineProps({ - placeholderText: String -}) +</script setup lang="ts"> +const props = defineProps<{ + placeholderText?: string +}>()packages/canvas/render/src/canvas-function/custom-renderer.ts (3)
5-13: Consider adding TypeScript type definitions for schema and props.The
defaultRendererfunction referencesschema.childrenandschema.propswithout explicit type definitions, which could lead to runtime errors if the shape ofschemachanges. Leveraging proper TypeScript interfaces (e.g.,Schema,Props) would make the code more robust and self-documenting.
14-18: Clarify the fallback rendering condition.When
entryis falsy and there are no child components (or the canvas is not active), the function returns[h(CanvasEmpty)]. This approach is likely correct, but consider documenting or clarifying the design intention, so future maintainers can quickly understand the empty state logic.
26-39: Consider separating i18n concerns from the rendering container.Wrapping everything in
tiny-i18n-hostis convenient for localization, but if multiple sections share different locales, you may need more granular control. For instance, a specialized i18n composition function could selectively wrap only relevant sub-trees.packages/canvas/render/src/page-block-function/schema.ts (3)
33-34: Consider replacing JSON-based deep clone with a more robust solution
JSON serializing and parsing for deep cloning may cause issues with certain data types (e.g., Date, RegExp). Consider a library-based or custom solution to handle these edge cases.- const newSchema = JSON.parse(JSON.stringify(data || schema)) + // Example: using lodash.cloneDeep or a custom deep clone function + import cloneDeep from 'lodash/cloneDeep' + const newSchema = cloneDeep(data || schema)
80-95: Robust error handling for accessor functions
The error notification logic is clear. However, consider providing more context (e.g., stack trace or a link to docs) to help users debug complex accessor errors more effectively.
100-133: Large returned object might benefit from smaller grouped modules
Returning a large object with many references can be less maintainable. Consider splitting or grouping logically related functionality for clarity.packages/canvas/render/src/material-function/scope-css-plugin.ts (2)
165-165: Remove the leading semicolon to satisfy lint rules
This line has been flagged by ESLint as an unnecessary semicolon.- ;(node as selectorParser.Node).spaces.after = '' + (node as selectorParser.Node).spaces.after = ''🧰 Tools
🪛 eslint
[error] 165-165: Unnecessary semicolon.
(no-extra-semi)
82-92: Warn about deprecated /deep/ combinator
Vue’s /deep/ selector is deprecated. Although the comment clarifies it, consider providing a more visible warning or fallback approach for long-term maintenance.packages/canvas/render/src/RenderMain.ts (2)
39-48: Consider enumerability and serialization for dataSourceMap
Object.definePropertyhere sets a getter fordataSourceMap. Ensure you handle cases where a consumer might attempt to serialize or merge this object, as it’s non-enumerable.
200-212: Enhance reusability for watchers
Your watchers handle schema changes and active states effectively. However, if logic grows, consider extracting them into composable functions for better testability.packages/canvas/render/src/data-function/parser.ts (1)
94-97: Recursive parse for JSX
The fallback to JSX parsing in the catch block is clever. Ensure that repeated parse attempts won’t degrade performance in large-scale usage.packages/canvas/render/src/render.ts (2)
45-45: Avoid assignments in short-circuit expressions
This expression uses an assignment within a short-circuit condition. It can be harder to read and may trigger lint errors.- isCustomElm && (child.props.slot = 'slot') + if (isCustomElm) { + child.props.slot = 'slot' + }🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
114-115: Replace delete operator for performance
Deleting properties can degrade performance. Consider setting them to undefined or using a new object without the property instead.- delete bindProps.className + bindProps.className = undefinedpackages/canvas/render/src/material-function/configure.ts (2)
1-1: Use consistent types for future maintainability.
DeclaringconfigureasRecord<string, any>is fine for flexibility. However, if you anticipate a more stable configuration structure in the long run, using a stricter type can help avoid unintended values.
2-4: Ensure robust type definitions for setConfigure.
Currently,setConfigureacceptsconfigureDatawith no explicit type annotation. Consider adding a type signature or generics if your configuration is expected to follow a structure.-export const setConfigure = (configureData) => { +interface ConfigureData { + [key: string]: any +} + +export const setConfigure = (configureData: ConfigureData) => { Object.assign(configure, configureData) }packages/canvas/render/src/canvas-function/controller.ts (2)
1-2: Prefer typed fields for centralized “controller” object.
Like withconfigure, consider replacingRecord<string, any>if you can define your controller shape precisely. This helps catch misuse at compile time.
7-7: Potential debug or tracing hook.
Consider adding a debug log or callback to track changes to the controller. This can aid in diagnosing issues where multiple modules overwrite each other’s data.packages/canvas/render/src/material-function/handle-scoped-css.ts (1)
4-6: Leverage async API or handle errors.
postcss(...).process(...)can also be invoked with asynchronous usage or error handling to prevent unhandled promise rejections. Consider using.process(content, { from: undefined })to remove file references, or handling any potential exceptions from plugin processing.export function handleScopedCss(id: string, content: string) { - return postcss([scopedPlugin(id)]).process(content) + return postcss([scopedPlugin(id)]) + .process(content, { from: undefined }) + .catch((error) => { + console.error('Error in handleScopedCss:', error) + throw error // rethrow to ensure the caller is aware + }) }packages/canvas/render/src/canvas-function/design-mode.ts (1)
11-13: Enforce valid design modes or add type checks.Currently,
setDesignModeaccepts any value, which may lead to unexpected states. Consider restricting the parameter to'design' | 'runtime'or adding runtime checks for invalid modes.packages/canvas/render/type.d.ts (1)
3-10: ReplaceRecord<string, any>with more precise types when possible.While this declaration extends the
Windowinterface in a flexible way, relying heavily onanycan reduce the benefits of TypeScript. Consider refining these property types where feasible.packages/canvas/render/src/builtin/CanvasRouterView.vue (2)
11-13: Consider a default value or validation for thenameprop.With only a type declaration, there's no enforced requirement or default. If this prop is optional, consider a sensible default or runtime validation for better flexibility.
14-15: Add a default value or runtime check forparamsto avoid undefined usage.If other components rely on accessing specific keys within
params, it may cause runtime errors ifparamsis undefined or null. A default value like an empty object{}would ensure more robust usage.props: { name: { type: String }, params: { type: Object, + default: () => ({}) } }packages/canvas/render/src/application-function/bridge.ts (1)
3-16: Type annotations and concurrency considerations.
- TypeScript Type Annotations: The
bridgeobject is currently untyped, which can lead to unclear usage. Consider introducing an interface or type forbridgeto ensure maintainability.- Concurrent Mutations: If this function is used across multiple modules, watch out for potential shared state mutations. You may want to adopt a reactive store if concurrency could become an issue.
packages/canvas/render/src/data-utils.ts (1)
8-12:newFnusage is acceptable but consider security implications.Creating functions dynamically can be powerful but risky if arguments come from untrusted sources. Validate inputs or confirm controlled usage.
packages/canvas/render/src/canvas-function/page-switcher.ts (2)
4-8: Check if partial updates ofICurrentPageare needed.Currently, all fields are required in
ICurrentPage. If partial updates are likely, consider optional properties to avoid forcing a redefinition of unchanging fields.
9-13: Inconsistent types when defaulting tonull.
pageIdis declared asstring | number, but is set tonullby default. TS will allow it, but semantically it's inconsistent. Consider allowingnullin the type or using a more suitable placeholder.export interface ICurrentPage { - pageId: string | number + pageId: string | number | null schema: any pageContext: IPageContext }packages/canvas/render/src/canvas-function/locale.ts (2)
1-4: Use descriptive comment or docstring for clarity
While the imports are straightforward, consider adding a brief docstring or comment explaining the high-level functionality oflocale.tsand how it integrates withI18nInjectionKey. This can help future maintainers understand why these specific imports are required.
10-11: Remove the unnecessary semicolon
A static analysis hint indicates that the semicolon in line 11 is unnecessary. Remove it to keep the code clean.- ;(locale as WritableComputedRef<unknown>).value = data.value + (locale as WritableComputedRef<unknown>).value = data.value🧰 Tools
🪛 eslint
[error] 11-11: Unnecessary semicolon.
(no-extra-semi)
packages/canvas/render/src/material-function/support-collection.ts (1)
1-1: Consider a typed map for future readability
collectionMethodsMapis declared as an untyped empty object. Consider specifying an explicit type (e.g.,Record<string, SomeDataSourceType>) to clarify usage and reduce potential runtime errors.packages/canvas/render/src/application-function/global-state.ts (1)
19-25: Optimize object spread in reduce
Spreading accumulators within a reduce callback can cause unnecessary overhead. If performance becomes a concern, consider pushing updates or mutating an existing object instead of creating new copies at each iteration.- const computedGetters = Object.keys(getters).reduce( - (acc, key) => ({ - ...acc, - [key]: new Func('return ' + getters[key])().call(acc, state) - }), - {} - ) + const computedGetters = {} + for (const key of Object.keys(getters)) { + computedGetters[key] = new Func('return ' + getters[key])().call(computedGetters, state) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/canvas/render/src/builtin/index.ts (1)
31-32: Ensure consistent export ordering
Although this is non-blocking, consider ordering imports and exports alphabetically or in a logical grouping reflecting their functionality. This helps with readability and maintainability.packages/canvas/render/src/builtin/CanvasRouterLink.vue (2)
13-14: Use explicit file naming conventions
For consistency in TypeScript Vue SFCs, consider adding a named export in the script block or naming the component to ensure consistency across your codebase.
25-30: Future-proof thetoprop
You have a TODO note indicating thattomight become an object in the future. Consider using a union type from the start, if appropriate, to prevent potential refactoring overhead.packages/canvas/render/src/page-block-function/state.ts (1)
14-15: Consider clarifying usage ofclear
The optionalclearparameter triggers a reset on the state, which is fine. However, consider adding doc comments or a more expressive function name to make the intended usage clearer.packages/canvas/render/src/application-function/data-source-map.ts (1)
29-47: Flexible data source design
Creating a dynamicloadmethod that resolves with precomputed data is a neat way to abstract data fetching. Just ensure real network requests or advanced logic can be integrated here if needed, so your pattern scales for bigger data.packages/canvas/.eslintrc.cjs (1)
28-28: AddingtypescriptpluginIncluding
'typescript'in the Babel plugins ensures transformations for TS-specific syntax. Check that your Babel configuration is minimal if you rely on type-checking fromtscor Vue’s build tools.packages/canvas/render/src/page-block-function/props.ts (1)
4-10: Proper structuring ofusePropsDefining
propsas a simple object and providingsetPropswith an optional clear parameter is a clear approach. This pattern helps ensure consistent data handling and easy resets.Consider if you need reactive state with Vue’s
reactiveorrefforprops, depending on reactivity needs in other parts of the code.packages/canvas/render/src/page-block-function/accessor-map.ts (1)
5-9: Defining accessor typesUsing
'getter' | 'setter'strongly codifies expected accessor behavior. Interfaces are well-defined, though consider marking fields optional if not always present.packages/canvas/render/src/application-function/utils.ts (1)
40-43: Verify error handling for generated functions.The generated function might throw runtime errors if the content contains invalid code or references unavailable variables in
context. Consider adding a try/catch block or a fallback mechanism.packages/canvas/render/src/lowcode.ts (1)
25-25: Avoid casting toanyfor better type safety.
Castinginject(I18nInjectionKey).globaltoanydefeats the benefit of TypeScript. Consider defining an explicit interface.packages/canvas/render/src/material-function/material-getter.ts (1)
72-80: EnsureregisterBlockfor newly used blocks.
When custom elements are created on the fly, make sure blocks are registered upfront (viaregisterBlock) to avoid timing issues in dynamic rendering scenarios.packages/canvas/render/src/canvas-function/canvas-api.ts (1)
46-80: Preserve strong typing on API methods.
All API functions are delegated tocurrentApi, but the types on each function are not enforced ifcurrentApiis undefined or partially implemented. Consider additional runtime checks or full coverage tests.packages/build/vite-config/src/default-config.js (1)
32-32: Add.tsand.tsxextension support looks good.
These additions align well with the project's migration to TypeScript. You may also consider including other extensions such as.mjsif your codebase uses them, but this is a minor point.packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)
175-196: Well-structured event subscriber with lifecycle cleanup
The IIFE sets up a clean subscription/unsubscription pattern and uses aSetfor callbacks to avoid registering duplicates. This approach is robust. If you foresee a large number of callbacks, consider bounding or systematically cleaning up inactive callbacks to avoid potential memory growth.
212-215: Controller method naming consistency
The methodaddHistoryDataChangedCallbackis cohesive withpostHistoryChangedinusePage.js, but watch out for naming clashes—both methods deal with history updates. Consider ensuring consistent naming conventions across modules, such aspostHistoryChangedvs.onHistoryChanged.packages/canvas/package.json (1)
41-41: Add usage context or rationale fortiny-engine-dsl-vue.A new dependency has been added. Provide short documentation or an internal note indicating the purpose and usage scenario, facilitating easier maintenance.
packages/plugins/materials/src/composable/useResource.js (1)
56-61: Ensure URL updates are consistent with browser navigation.The URL management implementation in the
finallyblock ensures that the page ID is always reflected in the URL, regardless of any errors during page initialization. However, consider the following:
- Using
pushStatecreates a new history entry. For page updates within the same context,replaceStatemight be more appropriate to avoid cluttering the browser history.- The URL update should handle cases where the
pageInfo.idis undefined or null.Consider this safer implementation:
} finally { const url = new URL(window.location) - url.searchParams.set('pageid', pageInfo.id) - window.history.pushState({}, '', url) + if (pageInfo.id) { + url.searchParams.set('pageid', pageInfo.id) + window.history.replaceState({}, '', url) + } usePage().postHistoryChanged({ pageId: pageInfo.id }) }packages/canvas/render/src/builtin/builtin.json (2)
135-154: Consider enhancing RouterView schema.While the minimal schema is functional, consider adding properties that would be useful for the designer:
nameproperty for named viewskeepAliveproperty for caching
155-220: LGTM! Well-structured RouterLink component.The RouterLink component is well-defined with essential navigation properties:
tofor navigation targetactiveClassfor styling active linksexactActiveClassfor exact route matchesHowever, there's a typo in the label for
exactActiveClass:- "zh_CN": "准确激活样式雷" + "zh_CN": "准确激活样式类"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (59)
package.json(1 hunks)packages/build/vite-config/src/default-config.js(1 hunks)packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(1 hunks)packages/canvas/.eslintrc.cjs(2 hunks)packages/canvas/DesignCanvas/src/DesignCanvas.vue(3 hunks)packages/canvas/package.json(1 hunks)packages/canvas/render/src/RenderMain.js(0 hunks)packages/canvas/render/src/RenderMain.ts(1 hunks)packages/canvas/render/src/application-function/bridge.ts(1 hunks)packages/canvas/render/src/application-function/data-source-map.ts(1 hunks)packages/canvas/render/src/application-function/global-state.ts(1 hunks)packages/canvas/render/src/application-function/index.ts(1 hunks)packages/canvas/render/src/application-function/utils.ts(1 hunks)packages/canvas/render/src/builtin/CanvasBox.vue(1 hunks)packages/canvas/render/src/builtin/CanvasRouterLink.vue(1 hunks)packages/canvas/render/src/builtin/CanvasRouterView.vue(1 hunks)packages/canvas/render/src/builtin/builtin.json(2 hunks)packages/canvas/render/src/builtin/index.ts(2 hunks)packages/canvas/render/src/canvas-function/CanvasEmpty.vue(2 hunks)packages/canvas/render/src/canvas-function/canvas-api.ts(1 hunks)packages/canvas/render/src/canvas-function/controller.ts(1 hunks)packages/canvas/render/src/canvas-function/custom-renderer.ts(1 hunks)packages/canvas/render/src/canvas-function/design-mode.ts(1 hunks)packages/canvas/render/src/canvas-function/global-notify.ts(1 hunks)packages/canvas/render/src/canvas-function/index.ts(1 hunks)packages/canvas/render/src/canvas-function/locale.ts(1 hunks)packages/canvas/render/src/canvas-function/page-switcher.ts(1 hunks)packages/canvas/render/src/context.js(0 hunks)packages/canvas/render/src/data-function/index.ts(1 hunks)packages/canvas/render/src/data-function/parser.ts(1 hunks)packages/canvas/render/src/data-utils.ts(1 hunks)packages/canvas/render/src/lowcode.ts(1 hunks)packages/canvas/render/src/material-function/configure.ts(1 hunks)packages/canvas/render/src/material-function/handle-scoped-css.ts(1 hunks)packages/canvas/render/src/material-function/index.ts(1 hunks)packages/canvas/render/src/material-function/material-getter.ts(1 hunks)packages/canvas/render/src/material-function/page-getter.ts(1 hunks)packages/canvas/render/src/material-function/scope-css-plugin.ts(1 hunks)packages/canvas/render/src/material-function/support-block-slot-data-for-webcomponent.ts(1 hunks)packages/canvas/render/src/material-function/support-collection.ts(1 hunks)packages/canvas/render/src/page-block-function/accessor-map.ts(1 hunks)packages/canvas/render/src/page-block-function/context.ts(1 hunks)packages/canvas/render/src/page-block-function/css.ts(1 hunks)packages/canvas/render/src/page-block-function/index.ts(1 hunks)packages/canvas/render/src/page-block-function/methods.ts(1 hunks)packages/canvas/render/src/page-block-function/props.ts(1 hunks)packages/canvas/render/src/page-block-function/schema.ts(1 hunks)packages/canvas/render/src/page-block-function/state.ts(1 hunks)packages/canvas/render/src/render.js(0 hunks)packages/canvas/render/src/render.ts(1 hunks)packages/canvas/render/src/runner.ts(2 hunks)packages/canvas/render/src/supportUmdBlock.ts(2 hunks)packages/canvas/render/type.d.ts(1 hunks)packages/canvas/vite.config.js(1 hunks)packages/plugins/block/src/Main.vue(1 hunks)packages/plugins/block/src/composable/useBlock.js(2 hunks)packages/plugins/materials/src/composable/useResource.js(2 hunks)packages/plugins/page/src/PageTree.vue(2 hunks)packages/plugins/page/src/composable/usePage.js(2 hunks)
💤 Files with no reviewable changes (3)
- packages/canvas/render/src/render.js
- packages/canvas/render/src/RenderMain.js
- packages/canvas/render/src/context.js
✅ Files skipped from review due to trivial changes (6)
- packages/canvas/render/src/data-function/index.ts
- packages/canvas/render/src/canvas-function/index.ts
- packages/canvas/render/src/material-function/support-block-slot-data-for-webcomponent.ts
- packages/canvas/render/src/material-function/index.ts
- packages/canvas/render/src/application-function/index.ts
- packages/canvas/render/src/page-block-function/index.ts
🧰 Additional context used
🪛 eslint
packages/canvas/render/src/canvas-function/locale.ts
[error] 11-11: Unnecessary semicolon.
(no-extra-semi)
packages/canvas/render/src/material-function/scope-css-plugin.ts
[error] 165-165: Unnecessary semicolon.
(no-extra-semi)
packages/canvas/render/src/page-block-function/context.ts
[error] 13-13: 'ref' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
packages/canvas/render/src/application-function/utils.ts
[error] 18-18: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/canvas/render/src/page-block-function/methods.ts
[error] 6-6: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/canvas/render/src/page-block-function/accessor-map.ts
[error] 13-13: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/canvas/render/src/application-function/global-state.ts
[error] 21-21: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/canvas/render/src/render.ts
[error] 45-45: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 122-123: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (69)
packages/canvas/render/src/canvas-function/custom-renderer.ts (1)
42-54: Use caution when storing the renderer in a local variable.
canvasRenderer is shared through getRenderer and setRenderer. While this simplifies swapping out the rendering logic, it could lead to unexpected behavior if multiple instances rely on distinct renderers. Consider implementing scoping for each usage context or verifying that only a single active renderer is used at a time.
packages/canvas/render/src/page-block-function/schema.ts (1)
62-62: Non-enumerable property caution
Defining a non-enumerable property can cause confusion when properties are merged or iterated over. Ensure this is intentional and won’t affect future merges or debugging.
packages/canvas/render/src/material-function/scope-css-plugin.ts (1)
60-60: Confirm the necessity of storing processed rules in a WeakSet
Using processedRules as a WeakSet can avoid memory leaks if rules go out of scope. Make sure rules are not needed again after plugin completion.
packages/canvas/render/src/RenderMain.ts (1)
161-163: Properly cleanup watchers
Good job cleaning up the history callback on unmount. Verify there are no other watchers created that may need cleanup.
packages/canvas/render/src/data-function/parser.ts (2)
174-174: Check for promise-like objects
Duck-typing with if (result.then) might catch objects that only define a .then property. Consider a more robust check or safeguard.
207-210: Default fallback when parser is not found
If typeParser is not found, the data returns as-is. Confirm that this behavior is intended and tested, especially for custom or new data types.
packages/canvas/render/src/render.ts (2)
90-92: Avoid overshadowing built-in events
Here, you attach onMouseover and onFocus to stop events. Ensure it won’t interfere with legitimate mouse or focus events needed by child components.
275-285: Ensure each schema node is set once
Invoking setNode(schema, parent) inside a loop respects repeated calls for looped items. Confirm it won’t overwrite or lose references for repeated nodes.
packages/canvas/render/src/canvas-function/controller.ts (1)
3-5: Add error handling or validations if needed.
Merging data into the controller without validation might introduce issues. If certain fields are required or restricted, consider adding checks. Otherwise, this is acceptable.
packages/canvas/render/src/material-function/handle-scoped-css.ts (1)
1-2: Import path improvements.
Great to see postcss and scopedPlugin used. Ensure the relative path './scope-css-plugin' is correct and that the file is properly exported from the plugin.
packages/canvas/render/src/builtin/CanvasBox.vue (2)
3-3: Empty slot usage is flexible.
Removing the placeholder is a valid simplification, letting consumers provide their own default content. This aligns well with the PR’s goal for more flexible subpage usage.
7-7: Nice to see TypeScript adoption.
Declaring <script lang="ts"> is consistent with the shift to TypeScript. Ensure that the rest of your .vue file, if it has complex logic, includes type annotations for props, data, and computed properties as needed.
packages/canvas/render/src/page-block-function/css.ts (1)
3-7: Consider adding type information for pageId and gracefully handling unavailable controller.
While the fallback logic using the nullish coalescing operator is sound, you might want to ensure:
pageIdis explicitly typed for better TypeScript compatibility.getBaseInfo()gracefully handles scenarios where the controller or thepageIdmight be unavailable or undefined.
packages/canvas/render/src/canvas-function/global-notify.ts (1)
7-7: Add type definitions and verify environment support.
Defining the shape of options can prevent runtime errors and improve clarity. Also, ensure BroadcastChannel is available in the target environment or that failures are gracefully handled.
packages/canvas/render/src/builtin/CanvasRouterView.vue (1)
2-2: Confirm the placeholder text meets design or i18n requirements.
Currently, the placeholder text "路由占位符" is hardcoded in Chinese. If this UI is intended for a multilingual audience, you may need an internationalization approach or an English-friendly fallback. Otherwise, this localized placeholder is acceptable.
packages/canvas/render/src/application-function/bridge.ts (1)
1-2: Confirm the reset import is sufficient for your use case.
Ensure that the reset function from data-utils is tested for all relevant scenarios. If you rely heavily on external side-effects of clearing bridge, confirm there's no concurrency issue or leftover references elsewhere.
packages/canvas/render/src/data-utils.ts (2)
1-2: Re-exporting parseFunction as generateFunction is clear and concise.
This approach is straightforward and helps maintain better naming consistency in your codebase.
4-6: Ensure the reset function aligns with expected usage.
reset deletes all properties from the input object, which can cause references in other modules to see an empty object. Confirm that this is the intended effect in all scenarios.
packages/canvas/render/src/canvas-function/page-switcher.ts (2)
1-3: Confirm imports and paths.
Ensure that IPageContext is correctly exported from ../page-block-function and that local development environment references are consistent.
15-19: Functionality for updating the current page looks correct.
The logic for setting pageId, schema, and pageContext is straightforward. Ensure there's a complementary getter or reactive usage pattern if other parts of the code rely on reading currentPage reactively.
packages/canvas/render/src/canvas-function/locale.ts (2)
6-9: Consider type safety for injected objects
inject(I18nInjectionKey).global is assumed to exist. If there's a scenario where inject returns undefined, the code could break. Consider adding a fallback or a type guard to ensure the injection is successful.
7-13: Confirm broadcast channel message structure
Ensure that data objects passed via useBroadcastChannel consistently contain the expected value properties. Build or update tests to verify that locale changes propagate correctly.
🧰 Tools
🪛 eslint
[error] 11-11: Unnecessary semicolon.
(no-extra-semi)
packages/canvas/render/src/material-function/support-collection.ts (1)
3-15: Validate function usage and handle corner cases
The generateCollection function works correctly for the typical case. A few points to consider:
- Ensure that
schema.componentName === 'Collection'is always correct for the intended usage path. - Validate that items in
schema.childrenconsistently containprops?.fetchData?.value. - Double-check what happens if
methodMatchreturns an empty string or if there's an unexpected format infetchData.value.
packages/canvas/render/src/page-block-function/methods.ts (1)
9-20: Assess concurrency and side effects
setMethods modifies the shared methods object in place before calling setContext. Ensure that no race conditions or reactivity timing issues break application logic. Thoroughly test these flows, especially if setMethods might be called concurrently or in rapid succession.
packages/canvas/render/src/application-function/global-state.ts (1)
6-34: Review constructor-based code evaluation
Using new Func('return ' + someString)() is powerful but can be prone to security concerns if the string is not carefully validated or sanitized. Confirm that the input for getters[key] is trusted or that the application context ensures no malicious code can be injected.
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/canvas/render/src/builtin/index.ts (1)
21-21: Great addition to the routing system
Importing CanvasRouterLink is consistent with how you import the other built-ins, and it aligns well with the new routing features discussed in the PR.
packages/canvas/render/src/builtin/CanvasRouterLink.vue (3)
1-11: Slot usage looks good
The slot exposing href, isActive, and isExactActive is well-structured, providing clear reactive data to child components. Looks good.
34-34: Use includes instead of indexOf
This comment was raised previously (), but it remains valid. Using includes is more readable and avoids magic numbers.
- const active = computed(() => pageAncestor?.length && pageAncestor.indexOf(props.to) > -1)
+ const active = computed(() => pageAncestor?.length && pageAncestor.includes(props.to))35-35: Edge case handling
When pageAncestor is empty (e.g., an undefined or zero-length array), pageAncestor[pageAncestor.length - 1] could be undefined. Currently, the code short-circuits with pageAncestor?.length && ..., but check if there's a scenario in which props.to might also be undefined, leading to an unintended truthy or falsy condition.
packages/canvas/render/src/page-block-function/state.ts (2)
7-7: Good use of shallowReactive
Storing the state with shallowReactive is efficient, avoiding deep watchers on large nested objects.
22-34: Generate watchers after data merging
Running watchEffect only after merging data ensures that cross-variable dependencies in the accessors are handled. This is a clever approach. Just be sure you’re not introducing any latency or unexpected reactivity loops.
packages/canvas/render/src/application-function/data-source-map.ts (1)
3-7: Interfaces are well-defined
These new interfaces (IDataSourceResult, etc.) bring clarity and type safety to data source handling. Good job on that.
packages/canvas/.eslintrc.cjs (4)
19-19: Adopting TypeScript ESLint Config Successfully
Including @vue/eslint-config-typescript in the extends array helps ensure best practices and linting support for TypeScript. This looks good for maintaining code quality in a Vue + TypeScript environment.
22-22: Switching the parser to @typescript-eslint/parser
Using the official TypeScript parser is essential to properly parse type syntax. This aligns with the move to TypeScript and should prevent parsing conflicts.
39-40: Enabling the TypeScript-specific unused vars rule
Disabling 'no-unused-vars' in favor of @typescript-eslint/no-unused-vars avoids conflicting rules and is the recommended approach for TS projects.
41-41: import/no-inner-modules is turned off
Some projects prefer controlling deep imports for clarity or tree-shaking reasons. Confirm that it's intentional to allow deeper imports from submodules.
Do you need to ensure partial imports won't cause bundling or maintainability issues?
packages/canvas/render/src/page-block-function/props.ts (6)
1-3: Efficient import usage
Bringing in reset and useAccessorMap clarifies that this file depends on external data management and accessor generation utilities. Ensure these imports remain lightweight to avoid circular dependencies.
13-17: Initialize props with accessor tracking
Declaring local props and accessorFunctions clarifies the function’s workflow. The approach to gather accessor functions indicates well-structured, step-by-step initialization.
18-21: Handling default values
Assigning defaultValue to the property ensures consistent behavior similar to Vue’s default prop logic. This is a straightforward approach for data initialization.
22-30: Generating accessor functions
Pushing generated get/set accessors to accessorFunctions before context is fully set up is a good separation of concerns. This approach prevents accidental side effects during initialization.
34-36: Applying the new props to the original props object
Calling setProps(props, true) after building the local props object ensures that resets and merges remain consistent. This design helps maintain a single source of truth.
38-43: Returning the full interface for managing props
Exposing props, initProps, getProps, and setProps comprehensively covers typical property lifecycle operations. Quality approach for reusability.
packages/canvas/vite.config.js (1)
38-38: Migrating render entry to TypeScript
Switching render entry to index.ts aligns with the TypeScript-first approach. Ensure that all references to index.js are correctly updated in any build or import paths.
packages/canvas/render/src/page-block-function/accessor-map.ts (6)
1-3: Correct imports for reactive and utility functionalities
Bringing in watchEffect, WatchStopHandle, and generateFunction sets up a foundation for dynamic, real-time accessor logic. Ensure globalNotify is well-scoped to handle warnings consistently project-wide.
11-16: Core logic in useAccessorMap
Wrapping the generation logic in a single function clarifies usage. The direct return shape fosters a well-modularized approach.
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
18-23: Maintaining separate maps for state and props
Defining individual Maps for state and props fosters clarity. This design helps keep watchers for each category separate without collisions.
24-30: Safely regenerating watchers
Properly clearing old watchers before creating new ones mitigates data confusion. Good practice to avoid potential memory leaks or stale references.
34-45: Robust error handling in watch effect
Catching errors and funneling them into a global notify preserves stability while providing user-facing warnings. Well-handled approach.
48-53: Returning accessor management functions
Exposing generateAccessor, generateStateAccessors, and the Maps encourages reusability and centralized accessor oversight. Solid design choice.
packages/canvas/render/src/application-function/utils.ts (1)
21-25: Validate clear logic in setUtils.
When clear is set to true, the existing utilities are reset. Confirm that resetting a potentially large utility object is the desired behavior, and ensure there's no data loss or unintended side effects for downstream consumers.
packages/canvas/render/src/supportUmdBlock.ts (1)
10-19: Global typings for window objects look good.
Declaring these properties on the global Window interface helps ensure type safety for modules that assume their existence. Just ensure these globals are used consistently across the project to prevent version mismatches.
packages/canvas/render/src/material-function/page-getter.ts (1)
87-87: Good use of String(id) for clarity.
Using String(id) to convert numeric or string identifiers ensures consistent return types in the pageChain. This aligns with a previous user request for more readable string conversion.
packages/canvas/render/src/runner.ts (1)
79-79: Resize observer dispatch usage looks good.
Including null or a structured detail object for your resize event is consistent with your custom dispatch approach. Just confirm that downstream listeners handle the event payload appropriately.
packages/canvas/render/src/lowcode.ts (2)
21-21: [Duplicate Comment] The usage of Record<string, any> is consistent with prior suggestions.
This aligns well with the earlier feedback from gene9831.
16-18: Validate new imports for cross-file synchronization.
These new imports (globalNotify, collectionMethodsMap, generateFn) look good, but ensure that any references to the old imports from ./render are removed throughout the codebase to avoid confusion or duplication.
✅ Verification successful
Let me try to find any references to these specific functions in the codebase to ensure proper synchronization.
Let me also check the specific files to understand the changes better:
No synchronization issues found with the imports
Based on the verification results:
- The functions
globalNotify,collectionMethodsMap, andgenerateFnare properly imported from their respective new locations (canvas-function,material-function, anddata-function). - No old imports from
./renderwere found in the codebase that would conflict with these new imports. - The functions are actively used across multiple files, and all imports reference the correct new locations:
globalNotifyis used in multiple files and imported fromcanvas-functioncollectionMethodsMapis imported frommaterial-functiongenerateFnis used in data-function related files
The imports have been properly synchronized across the codebase, and there are no lingering references to old import paths that need to be cleaned up.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for old imports from ./render across the codebase
rg --json --context 3 $'from \'\\.\\/render\''
Length of output: 141
Script:
#!/bin/bash
# Search for imports from ./render with proper escaping
rg "from ['\"]\\./render" -A 3
# Also search for imports of specific functions that were moved
rg "import.*?(globalNotify|collectionMethodsMap|generateFn)"
Length of output: 834
Script:
#!/bin/bash
# Search for usage of these functions
rg "(globalNotify|collectionMethodsMap|generateFn)\(" -A 2
Length of output: 2306
Script:
#!/bin/bash
# Check the content of related files
cat packages/canvas/render/src/render.ts 2>/dev/null || echo "File not found"
cat packages/canvas/render/src/canvas-function.ts
cat packages/canvas/render/src/material-function.ts
cat packages/canvas/render/src/data-function.ts
Length of output: 9638
packages/canvas/render/src/material-function/material-getter.ts (1)
37-39: Check for fallback logic if window.TinyLowcodeComponent is undefined.
When getNative is called and window.TinyLowcodeComponent is absent, the function returns undefined. Consider providing a fallback or error handling.
packages/canvas/render/src/canvas-function/canvas-api.ts (1)
42-44: Validate currentApi before assignment.
If setCurrentApi is called multiple times, confirm that reassigning currentApi doesn’t lead to inconsistent states or concurrency issues.
packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)
21-30: Importing new composition APIs and hooks
The additional imports for usePage and useMessage expand the component’s functionality to handle page management and message dispatch. This is a logical step to enable more sophisticated event handling and nested routing.
35-36: Expanded usage of meta APIs
Importing META_SERVICE and META_APP clarifies the external dependencies. Make sure to confirm that these service references are valid throughout your code and that they provide consistent responses for all use cases within nested routes.
packages/build/vite-config/src/vite-plugins/devAliasPlugin.js (1)
56-56: Alias updated to reference TypeScript entry
Switching from index.js to index.ts aligns with the broader TypeScript refactoring. Confirm that consumers of @opentiny/tiny-engine-canvas/render properly handle any additional type definitions or compile-time constraints introduced here.
packages/plugins/page/src/composable/usePage.js (1)
21-24: New postHistoryChanged function
Publishing the 'historyChanged' topic via useMessage() centralizes event handling for page history changes, which is essential for nested routing functionality. This makes the code more maintainable and event-driven.
packages/plugins/page/src/PageTree.vue (2)
102-103: Introduce unit tests or verification for new exports.
The addition of COMMON_PAGE_GROUP_ID and postHistoryChanged is beneficial. However, consider adding corresponding unit tests or ensuring that downstream usage is verified, so that any future refactoring or name changes become easier to catch.
144-144: Ensure consistent usage of postHistoryChanged.
You're calling postHistoryChanged immediately after updating the browser's history. Verify that all call sites (for example, in other components) align with this usage pattern to prevent any event-handling mismatches.
packages/plugins/block/src/Main.vue (1)
304-304: Validate block-level history updates.
This invocation (postHistoryChanged({ blockId: block.id })) appears correct, but ensure that the entire application correctly interprets block-level history changes. Also confirm that no events are lost if switching states in rapid succession between blocks.
packages/plugins/block/src/composable/useBlock.js (1)
32-34: Confirm message subscription or consumption for historyChanged.
The postHistoryChanged function publishes a historyChanged topic, yet it's unclear if consumers subscribe to this event in all relevant modules. Ensure that each module requiring history updates has subscribed appropriately to prevent invisible or lost messages.
packages/canvas/package.json (1)
48-50: Check for version conflicts and evaluate potential security patches.
The updates to add PostCSS and selector parser are typically safe, but verify that all versions are compatible. Also, ensure no known security vulnerabilities exist for these dependencies in your target environment.
package.json (1)
27-27: LGTM! Addition of TypeScript ESLint configuration.
The addition of @vue/eslint-config-typescript aligns with the PR's objective of refactoring JavaScript to TypeScript.
✅ Verification successful
TypeScript configuration and usage verified successfully
The addition of @vue/eslint-config-typescript is well-justified and properly integrated into the project:
- The project has TypeScript dependencies including
@types/nodeand@vue/eslint-config-typescript - There's extensive TypeScript usage with numerous
.tsfiles, particularly in thepackages/canvas/renderdirectory - The TypeScript setup appears to be properly configured for a Vue.js project with TypeScript support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify TypeScript configuration and dependencies
# Check if tsconfig.json exists
fd -t f "tsconfig.json"
# Check for other TypeScript-related dependencies
rg '"@typescript-eslint|"typescript"' -g 'package.json'
Length of output: 79
Script:
#!/bin/bash
# Let's check for TypeScript-related configurations and dependencies more thoroughly
echo "=== Checking package.json for TypeScript dependencies ==="
cat package.json | grep -E "typescript|@types|@vue/eslint-config-typescript"
echo -e "\n=== Checking for TypeScript configuration files ==="
fd -e json -x cat {} \; | grep -B2 -A2 "typescript"
echo -e "\n=== Checking for .ts files ==="
fd -e ts
Length of output: 2992
packages/plugins/materials/src/composable/useResource.js (1)
26-27: LGTM! New import for page management.
The addition of usePage from meta register aligns with the router subpage support feature.
packages/canvas/render/src/builtin/builtin.json (1)
610-643: LGTM! Well-structured router snippets.
The snippets for RouterView and RouterLink are well-organized:
- Properly categorized under "advanced" elements
- RouterLink includes a default Text child component
- Clear and descriptive labels in Chinese
packages/canvas/render/src/application-function/data-source-map.ts
Outdated
Show resolved
Hide resolved
…dd data error fallback/read protection, fix function params type declare
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
画布支持路由子页面嵌套，和实际的开发场景对应
What 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
Release Notes
New Features
postHistoryChangedmessaging functionality across various components for better history tracking.DesignCanvascomponent with new methods for page management.Bug Fixes
initPagefunction.Documentation
Chores