fix: don't use window in new WalletConnection

[chadoh]

Quick workaround to address #747 until the larger #748 is dealt with

[chadoh]

You can see that this works here: TENK-DAO/frontend-starter@bdfd70d

This can be used now by updating your package.json with:

"near-api-js": "AhaLabs/near-api-js#1.0.1"

[github-actions]

This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

Not stale!

[github-actions]

This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[volovyks]

@MaximusHaximus I think we can merge such a fix since we are not working on a better resolution now.

[github-actions]

This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

Not stale

[github-actions]

This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

this stale bot is a pain

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

OMG

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

😖

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

🥇

[github-actions]

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or this will be closed in another 7 days.

[chadoh]

Nooooooo

[agileurbanite]

@gutsyphilip @andy-haynes can you take a look at this PR?

[esaminu]

@agileurbanite @MaximusHaximus can this wait until we publish a near-api-nodejs package shortly that can be used in the server context? I think the user needs to be alerted of these issues as early as possible (i.e. when importing) rather than when using something. Also, making the current package 'safe' to import on the server context will involve applying this pattern in many other places including BrowserLocalStorageKeyStore among others.

For the purposes of SSR, like in Gatsby and Next.js I believe these issues can be addressed using dynamic imports like suggested here for example so that parts of the app that require use of the current package or the future near-api-browser are only invoked on the client side. Users implementing SSR will have to keep dynamically importing the browser package after we create a server side package if they would like to request login or signing of a transaction from a wallet.

[chadoh]

I understand this approach is not perfect but it's small and safe and fixes some important use cases. Why not merge it.

[chadoh]

Like, you can merge this now and do the bigger better thing later.

[ryancwalsh]

I agree. There have been so many people reporting this problem since Dec 2021 or earlier, and near-api-js has never been updated to address it. cc @agileurbanite

[esaminu]

@chadoh I think the best approach to take here is to treat the wallet export the same as what you did for the BrowserLocalStorageKeyStore export (i.e. export a fallback when window doesn't exist). From the WalletConnection class's perspective, we don't want the overhead of guaranteeing it will work without window.localStorage in context for current and future change sets. Maybe if there were values to render in the server here but all I see is getAccountId which will always return an empty string anyway without localStorage.

@ryancwalsh The first issue in #747 should be fixed the same way (just for the BrowserLocalStorageKeyStore usage in this case) and for the second issue regarding Buffer I was able to fix a similar issue on a basic gatsby starter by polyfilling Buffer via webpack (see gatsby-node.js) . In my case it was due to our usage of near/borsh-js which required Buffer. This is a good description of the issue and it may be something we address for browser specific packages so that Buffer does not have to be manually polyfilled cc @MaximusHaximus . We'll use #757 to track this.

The package separation will make which packages to import where more explicit but the same measures will have to be taken if you are importing browser modules in the server.

[esaminu]

Created a proposed solution in #896 . I think there's a genuine complaint here about devX in SSR stacks, we just have to balance that out with the behavior in Node.js as well

[esaminu]

Fixed in #896 Thanks guys! 🙂