fix: allow passing kit functions by reference

[chadoh]

What

Define methods using methodName = async () => {…} syntax, rather than async methodName() {…}

Why

This allows for better APIs when using StellarWalletsKit. For example, given a stellar-wallets-kit.ts file in my own project with:

const kit = new StellarWalletsKit({…});

export const signTransaction = kit.signTransaction;

And I then use this in my app:

const tx = await incrementor.increment();
const { result } = await tx.signAndSend({ signTransaction })

Today, the signAndSend will throw a runtime error:

TypeError: Cannot read properties of undefined (reading 'selectedModule')

This is because JavaScript's default behavior is coo coo bananas, no one understands it, and this ends up getting bound to undefined if you use the async methodName() {…} syntax and then pass methodName as a reference the way I did.

Today, in order to make my code work, I would need to export the whole kit and then change my signAndSend line to:

const { result } = await tx.signAndSend({
  signTransaction: async (xdr) => {
    return await kit.signTransaction(xdr);
  },
});

I don't like this because A) it's ugly and B) I don't think it's good practice to export the whole kit. Within my app, I want to have the ability to wrap interfaces like signTransaction, so that I can always make sure app-specific logic gets taken care of. Exporting all of kit adds more room for error.

The Fix

Using the arrow syntax with methodName = async (…) => {…} makes JS use similar this-binding logic to every other language, and makes my pass-by-reference use-case possible.

[chadoh]

I'd love to add a test, but didn't see any existing test files. If I'm being silly, please let me know where the tests are and I'll be sure to add one or two!

[earrietadev]

Hi there, I think this might not be the best solution because regular classes methods are defined in the prototype of the object while anonymous arrow functions will be duplicated every time someone extends the kit's class and that could have unexpected behaviors if someone uses the kit as a base for another library (tho I don't fully remember at the moment so I might be wrong).

Following your example, wouldn't be the same doing this but without changing all the methods in the kit?

const kit = new StellarWalletsKit({…});
export const signTransaction = kit.signTransaction.bind(this);
const tx = await incrementor.increment();
const { result } = await tx.signAndSend({ signTransaction });

I haven't used the tx.signAndSend method so let me know if my example is wrong

[chadoh]

Unfortunately, this doesn't work:

const kit = new StellarWalletsKit({…});
export const signTransaction = kit.signTransaction.bind(this);

I still get the same error. And you can see why: what is this, on that line? It's undefined; we're not inside a class, we're in the top level of a file.

that could have unexpected behaviors if someone uses the kit as a base for another library (tho I don't fully remember at the moment so I might be wrong)

That may or may not be a good point 😂

Do we think it's more important to make it easy to use the kit as a base for another library, or to make it predictable (and beautiful) to use the kit as-is within apps?

[earrietadev]

My bad, I meant export const signTransaction = kit.signTransaction.bind(kit);, I normally only use the bind function when I'm inside a class and I need to pass the current this scope down the road.

[chadoh]

Yes, bind(kit) will work.

I still find it a little clumsy and error-prone. To me, this sort of thing seems like the main use-case. I'm not sure it's worth requiring these bind calls, so that we can support a hypothetical use-case of using Stellar-Wallets-Kit as a base for another library. And when we're not even sure that the arrow syntax would break such a use-case!