fix: unable to undo after unique id#8238
Conversation
WalkthroughA comment line has been modified in the unique-id plugin to document the removal of an explicit meta call that previously prevented generated IDs from being added to the undo history. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses a bug with the undo feature in the editor's unique-id extension by commenting out a line that was preventing ID generation transactions from being added to the undo history. The commented-out line was causing issues with the undo functionality, so it has been temporarily disabled with an explanatory comment.
Key Changes
- Commented out
tr.setMeta("addToHistory", false)in the unique-id plugin'sappendTransactionhandler to fix undo feature issues
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Don't add ID generation to undo history | ||
| tr.setMeta("addToHistory", false); | ||
| // since its causing issue with undo feature we are commmeting it out for now |
There was a problem hiding this comment.
Spelling error: "commmeting" should be "commenting".
| // since its causing issue with undo feature we are commmeting it out for now | |
| // since its causing issue with undo feature we are commenting it out for now |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/unique-id/plugin.ts (1)
96-99: Clarify undo-history behavior in comments and fix typosRight now the comments are misleading and a bit hard to read:
- Line 96 still says “Don’t add ID generation to undo history”, but
tr.setMeta("addToHistory", false)is commented out, so ID-generation changes are now tracked in history (which seems intentional per the PR title).- Line 97 has typos (
its,commmeting) and doesn’t clearly describe what problem the meta flag caused.I’d suggest making the intent explicit and cleaning up the wording, e.g.:
- // Don't add ID generation to undo history - // since its causing issue with undo feature we are commmeting it out for now - // tr.setMeta("addToHistory", false); + // NOTE: We previously set `addToHistory = false` here to avoid adding ID generation to the undo history. + // This caused issues with the undo feature, so the meta flag is intentionally disabled for now. + // tr.setMeta("addToHistory", false);This keeps the historical context, explains why the flag is off, and avoids confusion for future readers about the actual undo behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/editor/src/core/extensions/unique-id/plugin.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/extensions/unique-id/plugin.ts
⏰ 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). (3)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.