polkadot-js / extension

Simple browser extension for managing Polkadot and Substrate network accounts in a browser. Allows the signing of extrinsics using these accounts. Also provides a simple interface for compliant extensions for dapps.
Apache License 2.0
975 stars 417 forks source link

Error with @polkadot/extension-dapp - "window is not defined" #1179

Closed enfipy closed 1 year ago

enfipy commented 1 year ago

What is the current behavior and expected behavior?

Currently, when you use @polkadot/extension-dapp with SSR - it fails on import with window is not defined.

Error code:

Code:
      47 | // just a helper (otherwise we cast all-over, so shorter and more readable)
    > 48 | const win = window; // don't clobber the existing object, but ensure non-undefined
         |             ^
      50 | win.injectedWeb3 = win.injectedWeb3 || {}; // true when anything has been injected and is available

Expected behavior: It should just work with import but fail on the run. It shouldn't just fail on import.

What is the motivation for changing the behavior?

Can't even import the @polkadot/extension-dapp package while on nodejs environment. I'm using SSR from astrojs and it doesn't work with the current latest @polkadot/extension-dapp.

My environment:

jacogr commented 1 year ago

It is not meant to be used in any Node.js (or Deno) environment. It is purely browser-only.

enfipy commented 1 year ago

@jacogr Sure, but why should it fall just by importing it? Can it use window only when making some function calls? For example, web3.js doesn't have this problem.

jacogr commented 1 year ago

import means it is executing in a JS context. The JS engine would parse the file and perform any variable definitions or top-level global code contained in that file. In this case it means setting up the global variables that are needed between the dapp and extension injectors.

enfipy commented 1 year ago

I do understand this, but I think just making it work as web3.js or ethers.js is a good intention. Perhaps I don't know something. If you want - you can close this issue.

polkadot-js-bot commented 1 year ago

This issue has been open for 21 days with no activity and is not labelled as an enhancement. It will be closed in 7 days.

enfipy commented 1 year ago

@jacogr Can I create a PR and just fix it with several ifs around these lines?

I still think that it shouldn't fail on just import as there're tons of frameworks like Nuxt/Next/SvelteKit/Astro (that can prerender Vue/React/Svelte components) and will fail on the import.

polkadot-js-bot commented 1 year ago

This issue has been open for 21 days with no activity and is not labelled as an enhancement. It will be closed in 7 days.

polkadot-js-bot commented 1 year ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.