feature: Integrating GameStop Wallet into onboard

[tylermanserGS]

Description

Checklist

• The version field in package.json is incremented following semantic versioning
• The box that allows repo maintainers to update this PR is checked
• I tested locally to make sure this feature/fix works
• This PR passes the Circle CI checks

[mahmud-bn]

Hi @tylermanserGS,
I'm working on integrating your PR but noticed a couple of inconsistencies on provider availability at page-load you might want to fix/look at.

Connecting the GameStop wallet worked fine initially, but after a while, I had to do multiple refreshes every 5-10 and logins to get it to show up and connect.

I've attached a sample of this issue:

Thanks

[tylermanserGS]

Hi @tylermanserGS, I'm working on integrating your PR but noticed a couple of inconsistencies on provider availability at page-load you might want to fix/look at.

Connecting the GameStop wallet worked fine initially, but after a while, I had to do multiple refreshes every 5-10 and logins to get it to show up and connect.

I've attached a sample of this issue:

Thanks

Hi @mahmud-bn,

This looks to be happening due to a race condition with the “window.gamestop” object. I put the GameStop wallet in as an injected wallet and looks to be failing to load half the time since the window[“gamestop”] does not exist when the injected WalletInit() is loaded in the app.

We list the “window.gamestop” object at document_start in our manifest and furthermore, we dispatch the event: “gamestop#initialized” as soon as the object is loaded. Are there any custom hooks or great ways to listen for this event before initializing certain injected wallets?

Any help is appreciated.

Thanks,
Tyler

[mahmud-bn]

Hi @tylermanserGS

Thanks for your reply,

To fix the race condition, here's what another wallet, Tally, had to do on their end to allow the injected provider to be available on page load.
https://github.com/tallycash/extension/pull/1228/files

They had to inject into the browser synchronously rather than async.

[Adamj1232]

@tylermanserGS wanted to check in and see how progress was moving? I helped Tally navigate this same issue with their wallet where it wasn't always available on the window object on app load. Please let me know if I can provide any support

[tylermanserGS]

Thanks, @Adamj1232 ! I have responded to a few different members in a BlockNative telegram channel. As we are using manifest v3 (while Tally is using v2), we can't inject inline scripts with v3 without doing a hacky workaround which requires near latest chrome version (which we can't force on users).

All extensions will have to upgrade to MV3 by Jan 2023. Aaron from your side is looking into a PR that allows an optional connectCallback to wallets that can be used to optionally connect to the provider. In our case, it would wait allow us to wait for event gamestop#initialized before attempting to hit up window.gamestop.

In the meantime, if you have other ideas or are in the midst of developing said custom hook, we would love to hear them!

[lnbc1QWFyb24]

Hey @tylermanserGS I have spent some time working on some solutions for handling async provider injections and hopefully have a fairly simple fix that works.

Adding any async functionality to the injected module would generally mean that we would need to either:

1. Release a breaking change to the injected module and require integrators to await the module init before initialising Onboard. This would make this wallet module unique in that respect and would also require the integrator to also pass in the user device that Onboard normally passes the module internally. I think this solution is less than ideal on many fronts.
2. Release a breaking change to the Onboard core module. This may be possible at a future date, but it could be a ways off as we would like to take the chance to implement some other breaking changes that we are just starting to think about now. So I don't think that this would be a timely solution.

The solution that I have come up with, is to delay when Onboard initialises the wallet modules internally to as late as possible. So instead of doing it at the time of Onboard init, it will do it just before rendering the connect modal. This means that in the typical case of the connectWallet function being called due to a user action (clicking a "connect" button), then there will have been plenty of time for the async injected module to be injected. The other case we need to consider is when the integrator implements the autoselect wallet pattern. In that case I have added a small delay to the connect function. Since this is automatically happening on load, it will not be perceived by the user, but should be enough time for the async provider to be injected.

This is the best compromise we can do at the moment, but I think it should work well.

I was not actually able to replicate the provider not being injected before starting this work like @mahmud-bn did, so I can't be sure that this fixes it.

Could you please take a look at this branch and test connecting gamestop wallet and then also refreshing the page to test reconnect. I have implemented the auto select wallet pattern, so it will try to connect to gamestop on page refresh. cc @mahmud-bn to also test again.

If you both could let me know if the issue is fixed on that branch, then we can go from there.

[tylermanserGS]

Hi @aaronbarnardsound ,

I tested out your branch this morning and can say it worked perfectly! The injected provider was able to load 100% of the time and I was able to see GameStop in the list without any issues (tested on Brave and Chrome). Appreciate you putting that branch together. Two small things:

1. The injected/src/package.json would need to have an updated version: "2.0.13-alpha.3" i believe. Also, maybe the "GameStop" keyword as part of that same file too. Lemme know if you want me to tack this on as a separate PR or anything.
2. Would you like me to go ahead and close this merge request now?

Thank you,
Tyler

[lnbc1QWFyb24]

Hey @tylermanserGS, awesome that is good to hear. Just to confirm you saw it working 100% of the time on page refresh as well?

I think the best way forward is for me to remove the Gamestop related code from that branch (i just put it in there for simple testing) and then create a PR for that. In the meantime if you can update this PR with the Game stop keyword in the injected module package.json file and merge upstream changes. I can then review this PR and get it merged as well.

Once all that is done, I can tee up a release.

[tylermanserGS]

Hi @aaronbarnardsound ,

To your point: I have tested your branch vigorously and refreshed many times in both Brave and Chrome. Worked 100% of the time for me. Thanks again for all your help! I have updated this PR with the GameStop keyword, new semantic version, and merged upstream changes.

Let me know if there is anything else you would like me to review/change.

Thanks,
Tyler

[lnbc1QWFyb24]

Looks good 🚀