โจ feat: reset random key to avoid ID conflicts during state parsing #103
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures that when JSON state is parsed into the editor, the global random key generator is reset based on existing node IDs to prevent future ID collisions. Sequence diagram for JSON state parsing with random key resetsequenceDiagram
participant JSONDataSource
participant Editor
participant EditorState
participant ParseSerializedNodeImpl
participant RandomKeyGenerator
JSONDataSource->>Editor: loadJSONState(dataObj)
Editor->>EditorState: withJsonState(state)
EditorState->>JSONDataSource: callback(state)
JSONDataSource->>ParseSerializedNodeImpl: $parseSerializedNodeImpl(dataObj.root, editor, true, state)
ParseSerializedNodeImpl-->>JSONDataSource: rootNode
JSONDataSource->>EditorState: access _nodeMap
JSONDataSource->>EditorState: iterate _nodeMap keys (exclude root)
JSONDataSource->>JSONDataSource: compute maxId from numeric keys
JSONDataSource->>RandomKeyGenerator: resetRandomKey(maxId + 1)
JSONDataSource->>EditorState: _nodeMap.set(rootNode.getKey(), rootNode)
Updated class diagram for JSONDataSource and random key managementclassDiagram
class DataSource
class JSONDataSource
class Editor
class EditorState {
Map~string, EditorNode~ _nodeMap
}
class EditorNode {
string getKey()
}
class RandomKeyGenerator {
resetRandomKey(startId number)
}
JSONDataSource --|> DataSource
JSONDataSource ..> Editor
JSONDataSource ..> EditorState
JSONDataSource ..> RandomKeyGenerator
Editor o--> EditorState
EditorState o--> EditorNode
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
|
๐ @ilimei |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The iteration over
state._nodeMap.keys()assumeskeys()returns something with.forEach, which is not true for standardMapiterators; consider usingfor (const key of state._nodeMap.keys())orArray.from(state._nodeMap.keys()).forEach(...)to avoid runtime errors. - When computing
maxId,Number(key)can yieldNaNfor non-numeric keys (beyond'root'), which will breakMath.max; it would be safer to skip keys that don't parse to a finite number before taking the max.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The iteration over `state._nodeMap.keys()` assumes `keys()` returns something with `.forEach`, which is not true for standard `Map` iterators; consider using `for (const key of state._nodeMap.keys())` or `Array.from(state._nodeMap.keys()).forEach(...)` to avoid runtime errors.
- When computing `maxId`, `Number(key)` can yield `NaN` for non-numeric keys (beyond `'root'`), which will break `Math.max`; it would be safer to skip keys that don't parse to a finite number before taking the max.
## Individual Comments
### Comment 1
<location> `src/plugins/common/data-source/json-data-source.ts:78` </location>
<code_context>
try {
const root = $parseSerializedNodeImpl(dataObj.root, editor, true, state);
+ let maxId = -1;
+ state._nodeMap.keys().forEach((key) => {
+ if (key === 'root') return;
+ maxId = Math.max(maxId, Number(key));
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `forEach` directly on `state._nodeMap.keys()` is likely incorrect because `Map.prototype.keys()` returns an iterator, not an array.
In JS/TS, `Map#keys()` returns an iterator, which doesnโt have `forEach`, so this will throw at runtime. Use a `for...of` loop over `state._nodeMap.keys()` or wrap with `Array.from(...)` if you specifically need `forEach` behavior.
</issue_to_address>Help me be more useful! Please click ๐ or ๐ on each comment and I'll use the feedback to improve your reviews.
commit: |
Member
|
โค๏ธ Great PR @ilimei โค๏ธ |
github-actions bot
pushed a commit
that referenced
this pull request
Jan 8, 2026
## [Version 3.9.0](v3.8.0...v3.9.0) <sup>Released on **2026-01-08**</sup> #### โจ Features - **misc**: Reset random key to avoid ID conflicts during state parsing. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's improved * **misc**: Reset random key to avoid ID conflicts during state parsing, closes [#103](#103) ([7119713](7119713)) </details> <div align="right"> [](#readme-top) </div>
Member
|
๐ This PR is included in version 3.9.0 ๐ The release is available on: Your semantic-release bot ๐ฆ๐ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
๐ป ๅๆด็ฑปๅ | Change Type
๐ ๅๆด่ฏดๆ | Description of Change
๐ ่กฅๅ ไฟกๆฏ | Additional Information