fix(dexie-cloud-addon): add configurable:true to awareness defineProperty

[liz709]

Problem

Object.defineProperty(provider, 'awareness', {...}) in createYHandler() was missing configurable: true, causing:

TypeError: Cannot redefine property: awareness

when the DexieYProvider 'new' event fires twice on the same provider instance. This reproducibly happens during Vite HMR in development.

Fix

Added configurable: true to the defineProperty call. This is a safe, non-breaking one-liner:

• Normal flow (single call) works identically
• HMR/duplicate scenario no longer throws
• The getter's if (awareness) guard already handles idempotency
• No production behavior change

Reported by @bennie.forss in Discord.

Summary by CodeRabbit

• Chores
  • Improved cloud sync stability by detecting and cleaning up stale awareness/session state when reconnecting, destroying outdated handlers and preventing reuse of prior state. The provider’s awareness can now be safely reconfigured, reducing duplicate or stale sync handlers and improving reliability during provider reinitialization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0462e337-02dd-4334-b19c-522b0de3f330

📥 Commits

Reviewing files that changed from the base of the PR and between 87c1cd9 and 8237f03.

📒 Files selected for processing (1)

• addons/dexie-cloud/src/yjs/createYHandler.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• addons/dexie-cloud/src/yjs/createYHandler.ts

---

📝 Walkthrough

Walkthrough

Handler now checks for an existing provider.awareness descriptor; if present it calls destroy() on the existing Awareness, removes the awarenessWeakMap entry for the doc, and then defines a new awareness accessor with configurable: true so it can be redefined later.

Changes

Cohort / File(s)	Summary
Yjs awareness handler addons/dexie-cloud/src/yjs/createYHandler.ts	If provider already has an awareness descriptor, treat it as a stale instance: call destroy() and delete the awarenessWeakMap entry for the doc. Then redefine provider.awareness as an accessor with configurable: true to allow future redefinition.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I found an old awareness, soft and slow,
I whispered "goodnight", then watched it go,
Cleared its trace where memories lay,
A fresh accessor hopped in to stay,
Configurable paws, ready for play. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main fix: adding configurable:true to the awareness defineProperty call in createYHandler, which is the core change addressing the TypeError during HMR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch liz/fix-yhandler-awareness-redefine

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

addons/dexie-cloud/src/yjs/createYHandler.ts (1)

27-37: Potential unnecessary initialization when cleaning up stale awareness.

When existingDescriptor exists but the old getter's awareness variable was never populated (user never accessed provider.awareness before HMR), accessing provider.awareness on line 32 invokes the OLD getter, which will lazily create an awareness with the OLD db instance, only to immediately destroy it.

A more efficient approach would check the descriptor type and avoid triggering the getter if possible:

♻️ Suggested optimization to avoid unnecessary initialization

     const existingDescriptor = Object.getOwnPropertyDescriptor(provider, 'awareness');
     if (existingDescriptor) {
       // Provider already initialized — likely a leaked handler from a previous db instance
       // (e.g. HMR where db.close() didn't fire). Destroy the stale awareness so the new
       // handler can take over cleanly.
-      const staleAwareness = provider.awareness as import('y-protocols/awareness').Awareness | undefined;
+      // Only attempt cleanup if there's an actual value (avoid triggering lazy init on old getter)
+      const staleAwareness = 'value' in existingDescriptor
+        ? existingDescriptor.value
+        : existingDescriptor.get?.call(provider);
       if (staleAwareness) {
         staleAwareness.destroy();
         awarenessWeakMap.delete(doc);
       }
     }

That said, this is an edge case during HMR, so the current implementation is acceptable if you prefer simplicity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@addons/dexie-cloud/src/yjs/createYHandler.ts` around lines 27 - 37, The code
currently reads provider.awareness which can invoke the old getter and lazily
re-create an Awareness; instead, check the existingDescriptor to avoid
triggering a getter: inspect existingDescriptor.value and existingDescriptor.get
and only access provider.awareness when the descriptor is a data property
(existingDescriptor.get is undefined) or when existingDescriptor.value is
already set (use existingDescriptor.value as the staleAwareness); if the
descriptor has a getter and value is undefined, skip destroying to avoid
initializing the old awareness. Use the symbols existingDescriptor,
provider.awareness, staleAwareness, awarenessWeakMap and doc to locate and
update the logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@addons/dexie-cloud/src/yjs/createYHandler.ts`:
- Around line 27-37: The code currently reads provider.awareness which can
invoke the old getter and lazily re-create an Awareness; instead, check the
existingDescriptor to avoid triggering a getter: inspect
existingDescriptor.value and existingDescriptor.get and only access
provider.awareness when the descriptor is a data property
(existingDescriptor.get is undefined) or when existingDescriptor.value is
already set (use existingDescriptor.value as the staleAwareness); if the
descriptor has a getter and value is undefined, skip destroying to avoid
initializing the old awareness. Use the symbols existingDescriptor,
provider.awareness, staleAwareness, awarenessWeakMap and doc to locate and
update the logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1be0bcba-d32b-4625-ad45-447bc0f8bd75

📥 Commits

Reviewing files that changed from the base of the PR and between 796949c and 87c1cd9.

📒 Files selected for processing (1)

• addons/dexie-cloud/src/yjs/createYHandler.ts