Don't make me import and initialize modules

[chadoh]

Am I ever expected to import and use things like FreighterModule in my own app? It doesn't seem like it. I think this is just an implementation detail of StellarWalletsKit itself. If that's true, then I would love to see:

1. Change the modules initialization param to accept a list of strings. These can be the *_ID strings that are currently used to set selectedWalletId.
2. Stop exporting all the *Modules altogether, as they seem like an implementation detail that leaked.
3. Changed allowAllModules exported function to a simple string, allModules

This would also allow me to code my initialization logic with more clear "exclude" rules, such as:

import { allModules, ALBEDO_ID, StellarWalletsKit } from '@creit.tech/stellar-wallets-kit'
const kit = new StellarWalletsKit({
  modules: allModules.filter(m => m !== ALBEDO_ID)
})

So far, I have found it much more desirable to opt out of specific wallets, while benefiting from the full list of allModules supported by StellarWalletsKit.

Alternatively, in addition to the changes above, you could also add an excludeModules option.

Caveats

• If you expect app developers to actually use *Modules in some way, you can still change the initialize parameters as described to make it easier and more expressive.
• If you expect wallet plugin developers to use these modules, you can export them from a specific file or entrypoint, so you don't clutter the interface for your main audience, which is app developers.
  • If this audience is still hypothetical, maybe don't clutter the interface for your main audience in service of them

[earrietadev]

Don't make me import and initialize modules

It was designed that way to be future proof without requiring to deploy a breaking change. For example both WalletConnect and Trezor wallets require the dev to include extra configuration when initializing the module and both were added after there were webs using the kit already live, app developers were able to add them by just importing the new module and specify the parameters required by them.

Change the modules initialization param to accept a list of strings. These can be the *_ID strings that are currently used to set selectedWalletId.

Changed allowAllModules exported function to a simple string, allModules

Stop exporting all the *Modules altogether, as they seem like an implementation detail that leaked.

That doesn't work for both Trezor and WalletConnect (and any future module that requires extra information before it initializes), also by manually importing modules instead of the kit automatically importing everything and just filtering by IDs is that, by doing it that, you avoid importing all the dependencies to your app if they are not needed.

For example WalletConnect dependencies are heavy and so it doesn't make sense to include them if you are not going to support it, another example are Ledger devices where your site needs to have a "Buffer" polyfill and so adding it automatically it means it will break all sites that doesn't have that (for example any app made using webpack 5+ or Angular which is more than 60% of the market).

[pselle]

That's helpful context as to why allModules isn't a set of strings.

In order to exclude modules (for whatever reason) maybe an excludeModules key that does take string arguments could be an idea?

[earrietadev]

@pselle that way will face the same issue because in order to filter by strings it means the library already loaded everything, instead I just added a new parameter for the allowAllModules so you can filter the default modules returned by that function while also allowing devs to include more complex modules like WalletConnect.

Here is an example of how to use it.

Let me know if that's enough for your use case.