capacitor-stripe-terminal icon indicating copy to clipboard operation
capacitor-stripe-terminal copied to clipboard

No way to destroy an instance

Open Irrelon opened this issue 2 years ago • 2 comments

Describe the bug Once instantiated it appears that the plugin will establish listeners for native events. If you create a new instance of the plugin and let go of any reference to the original, the internal events are still hooked to the native bridge and so an event asking for a connection token will fire on both the original instance and the new one.

The reason it would be useful to destroy an instance (and clear those listeners) is because we might want to switch to a different account in stripe and therefore pass a different stripe account id when doing a fetchConnectionToken, but what happens is that each previous instance gets the same event and calls the fetchConnectionToken for each instance. The last one to succeed calls sdk.setConnectionToken({ token }, null) and subsequent operation of the plugin is then error prone.

To Reproduce Instantiate a StripeTerminalPlugin.create() then do it again with a different fetchConnectionToken function.

Expected behavior Expect to be able to destroy previous instances created with StripeTerminalPlugin.create(). Either a destroy() method or some other method that at least resets listeners back to the initial state before being created. There may be further requirements other than just listeners, those just appeared to be the obvious standout issue - but I might be incorrect about them causing the issue described.

Irrelon avatar Sep 19 '22 19:09 Irrelon

You should not instantiate another instance of StripeTerminalPlugin pretty much ever. There maybe should be a destroy method but right now the correct way to switch accounts is by using clearCachedCredentials and then changing where fetchConnectionToken is getting its token. fetchConnectionToken should be defined in a way that you can change an external parameter (e.g. an account id of some sort) without having to pass in a new function.

Here's an extremely stripped down example of a service class that demonstrates that.

export class StripeTerminalService {
  public accountId: string
  public instance: StripeTerminalPlugin

  // use this method to initialize and set the account
  public async init(accountId: string) {
    // set the accountId so that fetchConnectionToken can use it
    this.accountId = accountId

    // only create a new instance of StripeTerminalPlugin if it hasn't already been initialized
    if (this.instance) return

    this.instance = await StripeTerminalPlugin.create({
      fetchConnectionToken: this.fetchConnectionToken.bind(this)
    })
  }

  // use this method to "log out" of an account
  public async destroy() {
    if (!this.instance) return

    // disconnect any connected readers
    await this.instance.disconnectReader()

    // clear cached connection token
    await this.instance.clearCachedCredentials()
  }

  private async fetchConnectionToken() {
    // use the accountId set in the init method to get your connection token
    const data = http.get('/api/connection-token', {
      accountId: this.accountId
    })

    return data?.secret
  }
}

nprail avatar Feb 28 '23 17:02 nprail

+1 on this. If you use the plugin in an environment that supports hot reloading each reload adds a new set of listeners. Not a major issue, but it would be convenient to have a destroy function.

Thanks for the great plugin!

create-signal avatar Apr 08 '23 01:04 create-signal