Skip to content

Conversation

SkinnnyJay
Copy link

@SkinnnyJay SkinnnyJay commented Sep 14, 2025

Changes being requested

Replace class private fields with module-scoped WeakMaps to prevent runtime crashes caused by JavaScript private field brand checks across duplicate SDK installations.

Core changes:

  • Replace class #private fields with WeakMap-based encapsulation in core components
  • Modify core/api-promise.ts, core/pagination.ts, and core/streaming.ts
  • Add minimal inline rationale comments explaining the WeakMap approach
  • Maintain existing encapsulation guarantees without public API changes

Test coverage:

  • Add regression tests to prevent future brand-check crashes
  • Verify the cross-copy method compatibility works correctly

Additional context & links

Bug: #1590

This addresses "TypeError: Cannot read private member" runtime crashes that occur when methods from different SDK copies interact, particularly in environments with duplicate SDK installations.

Problem:
JavaScript private field brand checks fail when instances cross module boundaries between different SDK copies, causing runtime errors in production environments.

Solution:
WeakMaps provide the same encapsulation benefits as private fields, but don't enforce brand checks, allowing instances from different SDK copies to interoperate safely.

Impact:

  • ✅ Prevents runtime crashes in duplicate SDK installation scenarios
  • ✅ Maintains existing encapsulation and security guarantees
  • ✅ Improves cross-copy method compatibility
  • ✅ No public API changes or breaking changes
  • ✅ Minimal performance impact

✅ I understand that this repository is auto-generated, and my pull request may not be merged

…eck crashes

- Replace JS private fields with module-scoped WeakMaps on hot paths:
  - core/api-promise.ts (APIPromise ↔ client)
  - core/pagination.ts (AbstractPage ↔ client)
  - core/streaming.ts (Stream ↔ client; tee preserves client)
  - client.ts (replace #encoder with private field; private baseURLOverridden)
- Add minimal inline rationale comments; no issue links in code.
- Remove temporary repros under tests/temp/.
- No public API changes.
- APIPromise: detached then/catch/finally do not throw private-field TypeError
- Pagination: AbstractPage.getNextPage guarded under detachment
- Streaming: Stream.tee remains stable under detachment
@SkinnnyJay
Copy link
Author

Ready for review..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant