-
-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Missing docs for npm authors - consistency with browser/module specifiers in package.json #272
Comments
Maybe this one can help to some extent ? |
Did you try using the "module" condition? |
Note that having both CJS and ESM being resolved is an ok thing to have. What specifically is the issue here? |
Hi @guybedford - yes I also tried the "module" condition but this gives a different/related error with browser + module
This is the line that breaks (this.storage = null != e && e.storage ? e.storage : new i(Lt(Lt({}, Ci), e?.storageOptions))), where import i from "@walletconnect/keyvaluestorage"; looking at the import map we get "@walletconnect/keyvaluestorage": "https://ga.jspm.io/npm:@walletconnect/keyvaluestorage@1.0.2/dist/cjs/browser/index.js", i.e. the package.json for "main": "dist/cjs/index.js",
"types": "dist/cjs/index.d.ts",
"unpkg": "dist/umd/index.min.js",
"browser": "dist/cjs/browser/index.js",
"react-native": "dist/cjs/react-native/index.js", esm versions of this package exist - but the package.json doesn't point to them with browser only I get:
This is the line that causes the error: this.provider = new Z.JsonRpcProvider( // rest of code where import * as l from "@walletconnect/jsonrpc-provider";
var S = "default" in l ? l.default : l;
var Z = S; the importmap looks like: "@walletconnect/jsonrpc-provider": "https://ga.jspm.io/npm:@walletconnect/jsonrpc-provider@1.0.12/dist/esm/index.js", and the final line of that file looks like: export{JsonRpcProvider,JsonRpcProvider as default}; so I don't see that this is a problem with jspm directly, But I didn't know how to approach this with Let me know if you need anything else. |
@s-cork this is a really thorough breakdown, nicely done. Yes this is a common issue with the I would suggest therefore looking into the fix on the The fact that jsonrpc-provider is being loaded as ESM and not CJS in this workflow then seems to be the issue. My guess on that is that there must be another package that is importing jsonrpc-provider as ESM somewhere in the dependencies, which is then forcing both to the ESM version. So tracking the issue down further would likely be working that out. This is something that might be possible to fix in the generator through scoping to have two versions being supported - both the ESM and CJS versions running together. If you are interested in figuring out a proper configuration I think it is clear you want everything to be ESM or CJS for the most part. Applying the "exports": {
"browser": "./main-browser-esm.js",
"default": "./main-node-esm.js"
} is probably the most advisable path. These configurations can be made via PR or we have the JSPM override service for making compatibility patches - https://github.com/jspm/overrides. But you likely still need to figure out which package is using jsonrpc-provider to determine that. It is possible to run JSPM generator with the debugging enabled to do more analysis on this and get some traces, let me know if you'd like further help with that kind of thing too. |
I am trying to use
@walletconnect/sign-client
and@web3modal/standalone
through JSPM:https://generator.jspm.io/#Y2VgYGBiLk/MyUktSc7Py0tNLmFwQOHqF2em5+km52Sm5pU4GOmZ6xkBFaQmGefmpyTm6BeXJOYB6fy8VKCcsZ45AJVr5qVQAA
However, I can't achieve consistency with the CommonJS and ESM versions of the modules. In the link above, I specify the "browser" field, but some of the exports are still pointing to
dist/esm/index.js
.e.g. the import map for
@walletconnect/jsonrpc-provider
points to the ESM version:"@walletconnect/jsonrpc-provider": "https://ga.jspm.io/npm:@walletconnect/jsonrpc-provider@1.0.12/dist/esm/index.js"
The package.json for this package has the following configuration:
https://github.com/WalletConnect/walletconnect-utils/blob/master/jsonrpc/provider/package.json
The "browser" field points to the ESM version, which causes issues since the code seems to expect a CommonJS module.
The code performs an import like
import * as l from "@walletconnect/jsonrpc-provider"
, followed byvar S = "default" in l ? l.default : l;
. This breaks the code later on because"default"
is the actual default export and not the module. Manually changing the import map todist/cjs/index.js
resolves the issue, but it's not ideal.If I specify both the "browser" and "module" fields, I encounter issues in the opposite scenario where module versions are not specified.
I am happy to follow up with the authors of
@walletconnect
, but I couldn't find any documentation on JSPM that I could point them to, such as a "best practice for npm library authors."The text was updated successfully, but these errors were encountered: