| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Migrating to TypeScript eases the maintenance and detects errors earlier. Simplify the content of the story for consistency with other stories.
WalkthroughRefactored the HelloWorld Storybook example to TypeScript, switched Graph construction to accept a plugins array (default plugins + optional RubberBandHandler), removed explicit parent parameters from vertex/edge creation, and updated documentation references accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Story as Story (Template)
participant Plugins as Plugins factory
participant Graph as Graph instance
participant Model as Graph Model
Story->>Plugins: getDefaultPlugins()
Plugins-->>Story: plugins[]
alt rubberBand enabled
Story->>Plugins: plugins.push(RubberBandHandler)
end
Story->>Graph: new Graph(container, undefined, plugins)
Story->>Graph: graph.batchUpdate(begin)
Story->>Graph: graph.insertVertex({ id, ... })
Story->>Graph: graph.insertVertex({ id, ... })
Story->>Graph: graph.insertEdge({ source, target })
Story->>Graph: graph.batchUpdate(end)
Graph->>Model: apply mutations (vertices/edges)
Note right of Graph: RubberBandHandler registered via plugins\nlistens for pointer events (if present)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
📜 Recent review details Configuration used: Organization UI Review profile: CHILL Plan: Pro 📥 CommitsReviewing files that changed from the base of the PR and between 525dd5e and 5fae99a. 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)packages/website/docs/tutorials/the-hello-world-example.md (1)63-79: Documentation examples inconsistent with the actual story code.
The code examples in this documentation still show the parent parameter being explicitly passed to insertVertex and insertEdge (lines 71-73), but the actual HelloWorld story code (lines 64-81 in HelloWorld.stories.ts) omits the parent parameter entirely. This inconsistency could confuse users following the tutorial.
Please update these documentation examples to match the simplified API usage demonstrated in the migrated story, or vice versa.
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between e05cde5 and 525dd5e.
📒 Files selected for processing (2)📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with tab width 2, trailing comma ES5, print width 90, and end of line auto.
Files:
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use single quotes in TypeScript/JavaScript code (Prettier config: singleQuote: true).
Files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 857 File: packages/core/src/view/plugins/SelectionHandler.ts:1081-1092 Timestamp: 2025-06-26T13:27:19.012Z Learning: User tbouffard prefers to keep PRs focused on their main objective and defer implementation improvements or logic fixes to future dedicated PRs. For maxGraph, refactoring PRs like Dictionary→Map migration should not include unrelated logic improvements even when valid issues are identified in the migrated code.
Learnt from: CR Repo: maxGraph/maxGraph PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T16:45:26.610Z Learning: To minimize bundle size, use `BaseGraph` and import only required features as shown in `ts-example-selected-features`
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 774 File: packages/ts-example-selected-features/package.json:11-13 Timestamp: 2025-04-22T16:34:40.309Z Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json (~5.8.2) and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` commands because the TypeScript dependency is hoisted in the monorepo structure.
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 774 File: packages/ts-example-selected-features/package.json:11-13 Timestamp: 2025-04-22T16:34:40.309Z Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` because it's available through this hoisted dependency in the monorepo structure.
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 849 File: packages/html/stories/DragSource.stories.js:98-101 Timestamp: 2025-06-13T07:48:10.300Z Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 826 File: packages/js-example-nodejs/src/index.cjs:64-69 Timestamp: 2025-05-13T12:54:55.231Z Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.
Applied to files:
Learnt from: CR Repo: maxGraph/maxGraph PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T16:45:26.610Z Learning: To minimize bundle size, use `BaseGraph` and import only required features as shown in `ts-example-selected-features`
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 791 File: packages/ts-example/vite.config.js:30-30 Timestamp: 2025-04-29T13:25:31.494Z Learning: In the maxGraph project, each example package (ts-example, ts-example-selected-features, ts-example-without-defaults) implements different use cases with varying features, resulting in different application sizes. Therefore, each package has its own specific chunkSizeWarningLimit value in its vite.config.js file, calibrated to its expected bundle size.
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 774 File: packages/ts-example-selected-features/package.json:11-13 Timestamp: 2025-04-22T16:34:40.309Z Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json (~5.8.2) and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` commands because the TypeScript dependency is hoisted in the monorepo structure.
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 774 File: packages/ts-example-selected-features/package.json:11-13 Timestamp: 2025-04-22T16:34:40.309Z Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` because it's available through this hoisted dependency in the monorepo structure.
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 598 File: packages/website/docs/manual/getting-started.md:70-70 Timestamp: 2024-12-15T18:19:56.236Z Learning: In code examples within the documentation, such as in `packages/website/docs/manual/getting-started.md`, we assume that the `graph-container` element exists and is an `HTMLElement`, and we avoid adding error handling for its initialization to keep the code simple.
Applied to files:
Learnt from: CR Repo: maxGraph/maxGraph PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T16:45:26.610Z Learning: Applies to packages/core/src/**/*.ts : Wrap multiple model changes in `batchUpdate()` to optimize graph updates
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 593 File: .github/workflows/create-github-release.yml:42-42 Timestamp: 2025-01-28T16:22:25.804Z Learning: In the GitHub release notes template for maxGraph, TODO comments are intentionally included to mark sections that must be reviewed by maintainers before publishing the release. These TODOs should not be removed or converted to validation steps as they are part of the release review process.
Applied to files:
Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 785 File: packages/core/src/view/style/register.ts:19-19 Timestamp: 2025-04-28T08:24:39.831Z Learning: In the maxGraph project, the Perimeter namespace is defined in the 'builtin-style-elements.ts' file using the statement 'export * as Perimeter from './perimeter';'. To access the Perimeter namespace in other files, they must import from './builtin-style-elements' instead of directly from './perimeter', as part of the tree-shaking optimization.
Applied to files:
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the './builtin-style-elements' file as a namespace export ('export * as Perimeter'), making this the correct import location for accessing the Perimeter namespace.
Applied to files:
packages/core/src/view/plugins/index.ts (1)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- getDefaultPlugins (46-55)
packages/website/docs/tutorials/the-hello-world-example.md (1)packages/html/stories/HelloWorld.stories.ts (5)23-23: LGTM! File extension updated correctly.
The reference has been properly updated to reflect the TypeScript migration of the HelloWorld story.
18-23: LGTM! Imports updated for plugin-based architecture.
The addition of getDefaultPlugins and the restructured imports support the new plugin-based graph initialization pattern.
55-57: LGTM! Plugin-based architecture implemented correctly.
The use of getDefaultPlugins() with conditional RubberBandHandler follows the modern plugin-based pattern for graph initialization.
60-60: LGTM! Graph constructor updated for plugin-based initialization.
The Graph constructor now correctly accepts the plugins array as the third parameter, with the model parameter set to undefined to use the default model.
63-82: LGTM! Simplified vertex and edge insertion.
The code correctly uses batchUpdate to group mutations and simplifies the API by omitting explicit parent parameters, relying on default behavior. This is a cleaner approach that aligns with the PR's goal of simplification.
50-50: Incorrect type annotation for Template parameters.
The Record<string, string> type annotation is incorrect. The args object contains properties like contextMenu and rubberBand (used on lines 53 and 57) which are boolean values, not strings. This type should be Record<string, any> or a more specific interface that accurately reflects the actual property types.
🔎 Proposed fix⛔ Skipped due to learnings-const Template = ({ label, ...args }: Record<string, string>) => { +const Template = ({ label, ...args }: Record<string, any>) => {Learnt from: tbouffard Repo: maxGraph/maxGraph PR: 720 File: packages/html/stories/Wrapping.stories.ts:37-37 Timestamp: 2025-03-19T06:56:13.709Z Learning: In Storybook stories for maxGraph, arguments are typed as strings (Record<string, string>) even when they represent boolean values in the argTypes definition. Boolean conversions like Boolean(args.someFlag) are used to convert string representations to actual boolean values.
Sorry, something went wrong.
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Migrating to TypeScript eases the maintenance and detects errors earlier.
Simplify the content of the story for consistency with other stories.
Summary by CodeRabbit
Updates
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.