Skip to content

refactor: migrate the Thread story to TypeScript#877

Merged
tbouffard merged 1 commit intomainfrom
refactor/migrate_thread_story_to_ts
Jul 22, 2025
Merged

refactor: migrate the Thread story to TypeScript#877
tbouffard merged 1 commit intomainfrom
refactor/migrate_thread_story_to_ts

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Jul 18, 2025 •
edited by coderabbitai bot
Loading

This eases the maintenance and detects error earlier.

In addition:

  • display the overlay tooltip
  • add a description to the story to clarify its purpose to the users
  • improve the JSDoc of CellOverlay and ObjectIdentity

Summary by CodeRabbit

  • Documentation

    • Improved and clarified JSDoc comments and formatting for better readability and accuracy.
    • Enhanced example code in documentation to use modern JavaScript syntax.

  • Style

    • Refined code formatting and structure for improved clarity and consistency.

  • New Features

    • Story example now displays descriptive context above the graph and enables tooltips for better user experience.

  • Refactor

    • Updated function and parameter naming for improved clarity in the story example.
    • Adjusted overlay toggling logic and interval for clearer demonstration.

@tbouffard
refactor: migrate the Thread story to TypeScript
5a60d18 
This eases the maintenance and detects error earlier.

In addition:
  - display the overlay tooltip
  - add a description to the story to clarify its purpose to the users
  - improve the JSDoc of CellOverlay and ObjectIdentity
@tbouffard tbouffard added the refactor Code refactoring label Jul 18, 2025
@coderabbitai
Copy link

coderabbitai bot commented Jul 18, 2025 •
edited
Loading

Walkthrough

This update refines JSDoc comments and formatting in multiple files, clarifies type annotations, and enhances a story example with improved UI structure and explicit typing. Minor method signature changes are present, but no core logic or control flow is altered.

Changes

File(s) Change Summary
packages/core/src/util/ObjectIdentity.ts Simplified and clarified the JSDoc for the ObjectIdentity class; removed @class tag and merged description.
packages/core/src/view/cell/CellOverlay.ts Switched to type-only import for ObjectIdentity, improved JSDoc formatting, updated toString method signature.
packages/html/stories/Thread.stories.ts Added type annotations, improved UI structure, updated overlay switching logic, and modernized parameter usage.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TemplateFunction
    participant Graph
    participant VertexA
    participant VertexB

    User->>TemplateFunction: Call with args
    TemplateFunction->>Graph: Create graph
    TemplateFunction->>Graph: setTooltips(true)
    TemplateFunction->>Graph: insertVertex({parent, value, position, size})
    TemplateFunction->>Graph: insertEdge({parent, value, source, target})
    TemplateFunction->>VertexA: Possibly set/remove overlay (every 7s)
    TemplateFunction->>VertexB: Possibly set/remove overlay (every 7s)
    TemplateFunction->>User: Return main div with description and graph
Loading

Possibly related PRs

  • docs: improve jsdoc of CellOverlay #846: Both PRs focus on improving and modernizing the JSDoc comments and formatting for the CellOverlay class and its methods without changing any core logic or functionality.

Suggested labels

documentation

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-07-18T10_43_10_324Z-debug-0.log

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3402cff and 5a60d18.

📒 Files selected for processing (3)
  • packages/core/src/util/ObjectIdentity.ts (1 hunks)
  • packages/core/src/view/cell/CellOverlay.ts (5 hunks)
  • packages/html/stories/Thread.stories.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings 
Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/AbstractGraph.ts:492-495
Timestamp: 2025-04-24T12:34:56.726Z
Learning: PR #776 focuses on refactoring to introduce AbstractGraph as a base class and moving code from Graph to AbstractGraph. Type improvements and other refinements should be addressed in future PRs to keep this one focused on the core architectural change.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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: tbouffard
PR: maxGraph/maxGraph#835
File: packages/html/stories/Orthogonal.stories.ts:114-117
Timestamp: 2025-05-26T12:34:54.318Z
Learning: The `null!` assertion pattern exists across multiple stories in the TypeScript migration and should be addressed systematically rather than in individual PRs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#740
File: packages/core/src/util/requestUtils.ts:79-106
Timestamp: 2025-03-31T08:32:17.055Z
Learning: PR #740 is focused on reorganizing code into namespaces (guiUtils and requestUtils) without changing any implementations. Implementation improvements should be suggested for future PRs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.

Learnt from: tbouffard
PR: maxGraph/maxGraph#715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the `fit` method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.

Learnt from: tbouffard
PR: maxGraph/maxGraph#582
File: packages/core/__tests__/view/handler/config.test.ts:0-0
Timestamp: 2024-11-24T17:28:08.023Z
Learning: In `packages/core/__tests__/view/handler/config.test.ts`, when writing Jest tests in TypeScript for configuration reset functions, small duplication is acceptable if it maintains test clarity and readability. Avoid refactoring tests in ways that could make them harder to understand, especially when future use cases might require specific checks.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.
packages/html/stories/Thread.stories.ts (9) 
Learnt from: tbouffard
PR: maxGraph/maxGraph#770
File: packages/core/src/view/cell/CellTracker.ts:99-101
Timestamp: 2025-04-18T07:02:31.931Z
Learning: In TypeScript, when implementing an interface, methods can have fewer parameters than declared in the interface due to TypeScript's structural typing system. Both omitting unused parameters entirely and prefixing them with underscore (e.g., _sender) are valid patterns in the maxGraph codebase.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.

Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/Graph.ts:85-92
Timestamp: 2025-04-24T12:34:10.366Z
Learning: The Graph class intentionally doesn't use collaborators (cellRenderer, selectionModel, view) from options in initializeCollaborators, as it's designed to replicate the previous implementation using only its factory methods for backward compatibility.

Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/core/src/view/style/edge/Segment.ts:26-35
Timestamp: 2025-04-29T13:29:45.122Z
Learning: In the maxGraph project, JSDoc `{@link}` references to symbols don't need to be imported in the current file to work correctly. TypeDoc can resolve symbols across the entire project namespace, not just within the current file, which is common in modern documentation systems and IDEs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.

Learnt from: tbouffard
PR: maxGraph/maxGraph#618
File: packages/website/docs/tutorials/graph.md:56-59
Timestamp: 2025-01-06T09:09:47.410Z
Learning: In the maxGraph library, the classes "Cell" and "GraphDataModel" are valid, while "GraphCell" and "GraphModel" do not exist.

Learnt from: tbouffard
PR: maxGraph/maxGraph#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.
🧬 Code Graph Analysis (1)
packages/html/stories/Thread.stories.ts (1)
packages/html/stories/shared/configure.js (1)
  • createMainDiv (53-63)
⏰ 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)
  • GitHub Check: build
  • GitHub Check: build (macos-14)
  • GitHub Check: build (windows-2022)
  • GitHub Check: build (ubuntu-24.04)
🔇 Additional comments (11)
packages/core/src/util/ObjectIdentity.ts (1)

24-26: LGTM! JSDoc documentation improved.

The simplified JSDoc comment is clearer and more direct while maintaining all the necessary information about the class functionality.

packages/core/src/view/cell/CellOverlay.ts (4)

24-24: Good TypeScript practice: type-only import.

Changing to a type-only import is appropriate since ObjectIdentity is only used as a type constraint in this file.

30-31: JSDoc improvement enhances clarity.

The updated JSDoc provides clearer information about the overlay functionality and event handling.

144-156: Example code modernized with better syntax.

The updated example code uses modern JavaScript syntax and clearer variable naming, making it easier to understand.

218-218: Explicit return type improves TypeScript clarity.

Adding the explicit return type string | null | undefined makes the method signature clearer and more type-safe.

packages/html/stories/Thread.stories.ts (6)

36-36: Good TypeScript practice: explicit type annotation.

Adding the explicit type annotation Record<string, string> improves type safety and code clarity.

38-45: Enhanced user experience with descriptive content.

The addition of descriptive text using createMainDiv provides better context for users understanding what the example demonstrates.

49-50: Tooltip functionality improves user interaction.

Enabling tooltips with graph.setTooltips(true) allows users to see the overlay tooltip when hovering, as mentioned in the PR objectives.

61-73: Modern object parameter syntax improves readability.

The migration to object parameter syntax with named properties (parent, value, position, size, etc.) makes the code more readable and maintainable compared to positional arguments.

76-77: Better naming convention improves code clarity.

Renaming the function from f to switchOverlay provides a more descriptive and meaningful name.

89-90: Improved timing and immediate display.

The longer interval (7 seconds) provides better user experience for observing the overlay switching, and the immediate call to switchOverlay() ensures the overlay is displayed right away.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 18, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@tbouffard tbouffard merged commit 67320e8 into main Jul 22, 2025
7 checks passed
@tbouffard tbouffard deleted the refactor/migrate_thread_story_to_ts branch July 22, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactor Code refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tbouffard