fix: structuredClone fallback for older Android WebView#1018
fix: structuredClone fallback for older Android WebView#1018
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts (1)
4-4: Migration cloning strategy preserved with deepCloneUsing
deepCloneforplugin.settings.choicesandmacrosbefore running the migration helpers keeps the “don’t mutate original settings in place” behavior, and the finalplugin.settings.choices = deepClone(choices)write remains safe, just slightly redundant. No new migration‑logic risks introduced.Also applies to: 52-52, 55-55, 58-58
src/utils/deepClone.ts (1)
1-118: Robust deepClone implementation; note object‑descriptor behaviorThe structuredClone‑first strategy with a cycle‑aware fallback covers the important built‑ins (Date, Array, Map/Set, RegExp, ArrayBuffer/views) and should be more than sufficient for the plugin’s settings and choice trees.
One behavioral difference vs native
structuredCloneis in the generic object branch: you currently clone only enumerable own string keys and always emit simple data properties, so non‑enumerable properties, symbol keys, and getters/setters are flattened or dropped. That’s fine for today’s usage (plain data), but if you ever feed richer class instances through this helper, this can subtly change behavior.If you want to more closely preserve object shapes while still using the same fallback, you can switch to cloning full property descriptors; for example:
- const prototype = Object.getPrototypeOf(value); - const cloned: Record<string, unknown> = Object.create(prototype); - seen.set(value, cloned); - for (const [key, nestedValue] of Object.entries(value)) { - Object.defineProperty(cloned, key, { - value: deepCloneFallback(nestedValue, seen), - writable: true, - enumerable: true, - configurable: true, - }); - } - return cloned; + const prototype = Object.getPrototypeOf(value); + const cloned: Record<string, unknown> = Object.create(prototype); + seen.set(value, cloned); + + const descriptors = Object.getOwnPropertyDescriptors(value as object); + for (const [key, descriptor] of Object.entries(descriptors)) { + const clonedDescriptor = { ...descriptor }; + if ("value" in clonedDescriptor) { + clonedDescriptor.value = deepCloneFallback( + clonedDescriptor.value, + seen, + ); + } + Object.defineProperty(cloned, key, clonedDescriptor); + } + return cloned;Not required for this PR, but it would make the fallback even closer to native structuredClone semantics if you start cloning more complex instances.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/gui/AIAssistantProvidersModal.ts(2 hunks)src/gui/MacroGUIs/ConditionalBranchEditorModal.ts(2 hunks)src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts(2 hunks)src/migrations/migrate.ts(2 hunks)src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts(2 hunks)src/migrations/setProviderModelDiscoveryMode.ts(2 hunks)src/services/choiceService.ts(2 hunks)src/services/packageExportService.ts(2 hunks)src/services/packageImportService.ts(3 hunks)src/settingsStore.ts(1 hunks)src/utilityObsidian.ts(2 hunks)src/utils/deepClone.test.ts(1 hunks)src/utils/deepClone.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/MacroGUIs/ConditionalBranchEditorModal.tssrc/migrations/incrementFileNameSettingMoveToDefaultBehavior.tssrc/utils/deepClone.test.tssrc/migrations/setProviderModelDiscoveryMode.tssrc/services/choiceService.tssrc/migrations/migrate.tssrc/utils/deepClone.tssrc/services/packageImportService.tssrc/settingsStore.tssrc/utilityObsidian.tssrc/services/packageExportService.tssrc/gui/AIAssistantProvidersModal.tssrc/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/MacroGUIs/ConditionalBranchEditorModal.tssrc/migrations/incrementFileNameSettingMoveToDefaultBehavior.tssrc/utils/deepClone.test.tssrc/migrations/setProviderModelDiscoveryMode.tssrc/services/choiceService.tssrc/migrations/migrate.tssrc/utils/deepClone.tssrc/services/packageImportService.tssrc/settingsStore.tssrc/utilityObsidian.tssrc/services/packageExportService.tssrc/gui/AIAssistantProvidersModal.tssrc/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/MacroGUIs/ConditionalBranchEditorModal.tssrc/migrations/incrementFileNameSettingMoveToDefaultBehavior.tssrc/utils/deepClone.test.tssrc/migrations/setProviderModelDiscoveryMode.tssrc/services/choiceService.tssrc/migrations/migrate.tssrc/utils/deepClone.tssrc/services/packageImportService.tssrc/settingsStore.tssrc/utilityObsidian.tssrc/services/packageExportService.tssrc/gui/AIAssistantProvidersModal.tssrc/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts
🧠 Learnings (2)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/utils/deepClone.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to src/**/*.{ts,tsx} : Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters or `tests/obsidian-stub.ts`
Applied to files:
src/utils/deepClone.test.tssrc/utilityObsidian.ts
🧬 Code graph analysis (12)
src/gui/MacroGUIs/ConditionalBranchEditorModal.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/migrations/incrementFileNameSettingMoveToDefaultBehavior.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/utils/deepClone.test.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/migrations/setProviderModelDiscoveryMode.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/services/choiceService.ts (2)
src/types/choices/IMacroChoice.ts (1)
IMacroChoice(4-7)src/utils/deepClone.ts (1)
deepClone(1-15)
src/migrations/migrate.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/services/packageImportService.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/settingsStore.ts (2)
src/quickAddSettingsTab.ts (2)
QuickAddSettings(14-54)DEFAULT_SETTINGS(56-91)src/utils/deepClone.ts (1)
deepClone(1-15)
src/utilityObsidian.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/services/packageExportService.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/gui/AIAssistantProvidersModal.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts (1)
src/utils/deepClone.ts (1)
deepClone(1-15)
🪛 ast-grep (0.40.0)
src/utils/deepClone.ts
[warning] 62-62: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value.source, value.flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (11)
src/gui/MacroGUIs/ConditionalBranchEditorModal.ts (1)
6-6: LGTM! Centralized cloning utility integrated correctly.The migration from a local
cloneCommandsfunction to the project-widedeepCloneutility is appropriate and ensures consistent cloning behavior with Android WebView fallback support.Also applies to: 36-36
src/migrations/migrate.ts (1)
12-12: LGTM! Critical migration backup now has Android WebView support.Replacing
structuredClonewithdeepClonefor the migration backup ensures that settings backups work on older Android WebView environments wherestructuredClonemay be unavailable.Also applies to: 42-42
src/services/packageExportService.ts (1)
19-19: LGTM! Package export now supports older Android WebView.The switch from
structuredClonetodeepClonewhen cloning choices for package export ensures compatibility with environments wherestructuredCloneis unavailable.Also applies to: 68-68
src/migrations/setProviderModelDiscoveryMode.ts (1)
4-4: LGTM! Provider migration now supports older Android WebView.Replacing
structuredClonewithdeepCloneensures this migration can clone provider configurations on older Android WebView environments.Also applies to: 29-29
src/services/choiceService.ts (1)
21-21: LGTM! Choice duplication now supports older Android WebView.The replacement of
structuredClonewithdeepClonewhen duplicating macro choices ensures compatibility with older Android WebView environments.Also applies to: 53-53
src/utilityObsidian.ts (1)
24-24: LGTM! Utility function now supports older Android WebView.Replacing
structuredClonewithdeepClonein theexcludeKeysutility function ensures it works on older Android WebView environments wherestructuredClonemay be unavailable.Also applies to: 949-949
src/settingsStore.ts (1)
4-4: LGTM! Critical settings initialization now supports older Android WebView.This change is essential for the PR's goal. The settings store initialization with
deepClone(DEFAULT_SETTINGS)ensures the plugin can load on older Android WebView environments wherestructuredCloneis unavailable (addressing issues #786 and #777).Also applies to: 10-10
src/migrations/mutualExclusionInsertAfterAndWriteToBottomOfFile.ts (1)
7-7: LGTM! Migration cloning now supports older Android WebView.Both cloning operations (choices and macros) are correctly updated from
structuredClonetodeepClone, ensuring this migration works on older Android WebView environments.Also applies to: 52-52, 55-55
src/gui/AIAssistantProvidersModal.ts (1)
9-9: Provider edit cloning and reload wiring look soundImporting
deepCloneand using it to snapshot the provider before entering edit mode keeps Cancel/Save semantics intact while avoidingstructuredCloneon older environments. Thereload()call inside the Edit handler correctly re-renders only afterselectedProviderand its clone are set, without introducing new state bugs.Also applies to: 105-112
src/services/packageImportService.ts (1)
23-23: deepClone integration for package import looks correctSwitching from
structuredClonetodeepCloneforexistingChoicesand eachentry.choicepreserves the previous “work on copies” behavior while adding the Android‑safe fallback. The subsequent ID remapping and tree mutation still operate only on clones, so neitheroptions.existingChoicesnorpkg.choicesare mutated unexpectedly.Also applies to: 263-263, 276-279
src/utils/deepClone.test.ts (1)
1-67: Deep clone tests give solid fallback coverageThe suite exercises the key behaviors: missing
structuredClone, circular references, throwingstructuredClone, and class instances, while restoring the original global after each test. This provides good regression protection for the new utility and its Android‑driven fallback path. Based on learnings, this aligns well with the “add regression coverage for bug fixes” guideline.
## [2.9.3](2.9.2...2.9.3) (2025-12-15) ### Bug Fixes * add structuredClone fallback for Android ([#1018](#1018)) ([d1a3eed](d1a3eed))
|
🎉 This PR is included in version 2.9.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🚀 Release has been published: v2.9.3 |
Fixes Android load failures where
structuredCloneis missing/throws (seen in #786 and #777).deepClone()helper that usesstructuredClonewhen available, with a safe fallbackstructuredClone(...)calls in runtime code paths (settings init, migrations, package import/export, etc.)Fixes #786
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.