Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Handling Multiple Init's #163

Open
andrewpeters9 opened this issue Feb 12, 2023 · 0 comments
Open

Handling Multiple Init's #163

andrewpeters9 opened this issue Feb 12, 2023 · 0 comments

Comments

@andrewpeters9
Copy link

andrewpeters9 commented Feb 12, 2023

Issues:

  1. LedgerBridgeKeyring can accidentally be initialised twice on the same client, resulting in silent failures (e.g. messages be either duplicated or resolved prematurely/incorrectly).
  2. Now that set-up logic is being moved to a separate init function, there's a possibility that the same instance is init'ed twice, and thus requires appropriate handling and fail-safes
  • setupIframe to throw an error if it detects a duplicate iframe
  • this.currentMessageId to be removed and replaced with nanoid
    • Ensures that two different clients sending messages to the same bridge URL will always have a unique (instead of being 0-indexed and incrementing)
    • Reduces code complexity
  • Prevent this.bridgeUrl from being null before the iframe is created, or at least update the iframe if the keyring is deserialized with a new bridgeUrl
  • Safe-proofing delayed promise: Safe-Proofing delayedPromise #162
  • this._eventListener will be mutated if the keyring is init'ed twice, which would interfere with:
    • window.removeEventListener('message', this._eventListener)
    • Solution to this may be to just remove this._eventListener completely, and replace it with something like this:
type WindowEventHandlerTuple = readonly [
    keyof WindowEventHandlersEventMap,
    (this: WindowEventHandlers, ev: WindowEventHandlersEventMap[keyof WindowEventHandlersEventMap]) => any
];

class LedgerBridgeKeyring extends EventEmitter {
    _windowEventListeners: ReadonlyArray<WindowEventHandlerTuple> = [];


  _setupListener () {
    const messageListener = ({ origin, data }) => {
      // ...
    }
    
    this._addWindowEventListener(['message', messageListener])
  }
  
  _addWindowEventListener(windowEventHandler:  WindowEventHandlerTuple){
     window.addListener(...windowEventHandler);
     this._windowEventListeners = [...this._windowEventListeners, windowEventHandler];
  }
  
  destroy(){
    for(const listener of this._windowEventListeners){
      window.removeListener(...listener);
    }
  }
} 
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant