-
Notifications
You must be signed in to change notification settings - Fork 4
[ENG-598] Data persistence for tldraw #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes introduce file persistence and editor state management to the Tldraw preview component. The component now receives plugin and file props, implements debounced saving of editor state to file, and manages UI/editor state synchronization. Utility functions for file content replacement and TLData updates are added, and the React root creation is guarded against missing file references. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TldrawPreviewComponent
participant FileSystem
participant Plugin
User->>TldrawPreviewComponent: Edit canvas
TldrawPreviewComponent->>TldrawPreviewComponent: Debounced save triggered on change
TldrawPreviewComponent->>FileSystem: Serialize and save editor state between delimiters
FileSystem-->>TldrawPreviewComponent: Confirm write or return error
alt Save error
TldrawPreviewComponent->>FileSystem: Attempt recovery from file
end
User->>TldrawPreviewComponent: Visibility changes
TldrawPreviewComponent->>TldrawPreviewComponent: Update editor focus and pen mode
Estimated code review effort3 (~45 minutes) 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/obsidian/src/components/TldrawView.tsx (1)
136-150: Avoid creating unused React roots to prevent memory leaks.Creating a React root without rendering it can lead to memory leaks. Consider returning
undefinedinstead of the unrendered root whenthis.fileis not available.Apply this diff to fix the issue:
private createReactRoot(entryPoint: Element, store: TLStore) { + if (!this.file) return undefined; const root = createRoot(entryPoint); - if (!this.file) return root; root.render( <React.StrictMode> <TldrawPreviewComponent store={store} plugin={this.plugin} file={this.file} /> </React.StrictMode>, ); return root; }Then update the caller at line 184 to handle the undefined case:
const root = this.createReactRoot(container, this.store); if (root) { this.reactRoot = root; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/obsidian/src/components/TldrawView.tsx(1 hunks)apps/obsidian/src/components/TldrawViewComponent.tsx(3 hunks)apps/obsidian/src/utils/tldraw.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/obsidian/src/utils/tldraw.ts (3)
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-07-19T22:34:16.794Z
Learning: Applies to apps/obsidian/package.json : Prefer existing dependencies from package.json when adding or using dependencies in the Obsidian plugin
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: In the discourse-graph codebase, types.gen.ts contains automatically generated database function type definitions that may have reordered signatures between versions. This reordering is expected behavior from the code generation process and should not be flagged as an issue requiring fixes.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-08T14:51:55.299Z
Learning: DiscourseGraphs maintainers prefer not to add narrowly typed definitions for individual process.env keys to avoid unnecessary coupling.
🧬 Code Graph Analysis (3)
apps/obsidian/src/components/TldrawView.tsx (1)
apps/obsidian/src/components/TldrawViewComponent.tsx (1)
TldrawPreviewComponent(19-181)
apps/obsidian/src/utils/tldraw.ts (1)
apps/obsidian/src/index.ts (1)
DiscourseGraphPlugin(25-287)
apps/obsidian/src/components/TldrawViewComponent.tsx (3)
apps/obsidian/src/index.ts (1)
DiscourseGraphPlugin(25-287)apps/obsidian/src/utils/tldraw.ts (2)
updateFileData(136-146)replaceBetweenKeywords(126-134)apps/obsidian/src/constants.ts (3)
TLDATA_DELIMITER_START(63-64)TLDATA_DELIMITER_END(65-66)DEFAULT_SAVE_DELAY(72-72)
mdroidian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested in our 1:1, noted by you inline.
3b3c292 to
aac5678
Compare
059f473 to
41ca0d0
Compare
mdroidian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff.
RE: IDE linting, if you weren't seeing those lint errors we can postpone fixed them to before merge to main. But in the future let's try to fix new lint warnings that we create on the spot.
RE: package-lock.json
This is slightly concerning, as there were no package.json changes. I'm assuming this was missed from some previous PR? If not, I'd track down where these changes are coming from.
5a4c622 to
a66d28b
Compare
| const newData = getTLDataTemplate({ | ||
| pluginVersion: plugin.manifest.version, | ||
| tldrawFile: createRawTldrawFile(currentStore), | ||
| uuid: window.crypto.randomUUID(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trangdoan982 Just noticed, since we are generating a new uuid on each saveChanges, stringifiedData === lastSavedDataRef.current will always be false.
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments
* [ENG-495] Tldraw obsidian setup (#285) * cleaned * sm * address PR comments * [ENG-598] Data persistence for tldraw (#303) * data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments * [ENG-624] TLDraw Obsidian asset store (#326) * current state * works now * clean up * address PR comments * address PR reviews * cleanup * fix styling issues * address PR comments * correct styles * [ENG-599] Discourse node shape (#341) * current state * works now * clean up * address PR comments * address PR reviews * fix styling issues * latest progress * update new shape * shape defined * address PR comments * sm address PR review * current progress * reorg * address other PR comments * clean * simplify flow * address PR comments * [ENG-604] Create node flow (#387) * eng-604: create node flow * pwd * [ENG-658] Add existing node flow (#389) * eng-658-add-existing-nodes-flow * address PR comments * small changes * [ENG-601] Create settings for canvas and attachment default folder (#338) * add new settings * small add * ENG-600: Discourse Relation shape definition (#408) * ENG-605: Add new relation flow (#411) * [ENG-603] Add existing relations (#412) https://www.loom.com/share/3641f2a642714b0d849262344e8c6ee5?sid=0614c657-e541-4bfd-92df-9b1aa60945b6 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added a Relations overlay on the canvas that shows a “Relations” button when a discourse node is selected. - Introduced a Relations panel to view and manage relations for the selected node, including adding or removing links, with clear loading/error states. - Overlay appears above the canvas without disrupting existing tools. - Chores - Consolidated relation-type lookup into shared utilities and updated imports. No user-facing changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * [ENG-844] Add color setting for relation types (#429) * add color setting * address PR reviews * address PR commens * fix icons * ENG-812 Update of database cli tools (#401) * eng-812 : Update database cli tools: supabase, vercel, cucumber. * newer cucumber constrains node * [ENG-495] Tldraw obsidian setup (#285) * cleaned * sm * address PR comments * [ENG-598] Data persistence for tldraw (#303) * data persistence to the file * error handling * address PR comments * address some PR comments * address other PR comments * address PR comments * switch to pnpm * delete wrong rebase file * fix pnpm lock * fix type checks * address all the PR comments * delete redundant files * fix types * shift click to open file on the right split (#485) * address PR comments * final lint cleanup --------- Co-authored-by: Marc-Antoine Parent <maparent@acm.org>
https://www.loom.com/share/70ce37001ff14852b52930db9f3d27be?sid=b7cf0506-9498-419a-b4cf-31ce95e93e37
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores