paritytech / capi

[WIP] A framework for crafting interactions with Substrate chains
https://docs.capi.dev
Apache License 2.0
104 stars 9 forks source link

Compatibility with older stable Node.js versions #1162

Closed statictype closed 1 year ago

statictype commented 1 year ago

I'm creating this issue because according to the provided description, Capi seems to be operating solely on the latest version of Node.js.

Note: The minimum supported Node version is 20, as we require the Web Crypto API be accessible from globalThis.crypto for the sake of browser compatibility.

While leveraging the most recent features of Node.js has its benefits, I believe ensuring backward compatibility with older versions of Node.js would significantly enhance the usability and adoption of this library.

A good portion of the Node.js community still uses older versions due to various reasons such as system constraints, stability considerations, or simply company policies. By supporting these versions, the library would be accessible to a larger user base.

New Node.js versions, while offering new features, might also introduce new bugs or issues. Supporting older, stable versions can help provide a reliable solution for all users.

I understand that it might be tempting to leverage the newest Node.js features, but I believe the benefits of backward compatibility significantly outweigh the potential downsides.

Furthermore, I would appreciate some clarification on the term "for the sake of browser compatibility" mentioned in the description. There appears to be an implication that Capi may not be designed for execution within a browser environment. I would be grateful if you could verify this presumption and, if it is indeed the case, elucidate the preferred environment for Capi.

Could you also clarify if Capi is supporting the Node 20 version, or specifically the 20.3 version as described in a previous issue?

Looking forward to hearing your thoughts on this!

ryanleecode commented 1 year ago

I've raised this issue internally myself, and you do raise a good point that we are limiting our current user base because of a higher node version constraint. However it is important to remember that blockchain, crypto, web3, polkadot, capi, etc... are all bleeding edge technologies that will only gain mainstream adoption years, if not decades from now. Users who want to use Capi today, need to understand the constraints of operating on the bleeding edge and adjust their development environments accordingly -- otherwise they will need to wait.

My second point would be, it is not our intention to force users to use the most recent version of Node.. i.e. if node 22 came out today we wont not force our users to use node 22. It just so happened that the web crypto API was marked as stable in Node 20, so we needed to use node 20. And later this year Node 20 will be the active release anyways, so as time goes on this will be less of an issue.

Furthermore, I would appreciate some clarification on the term "for the sake of browser compatibility" mentioned in the description. There appears to be an implication that Capi may not be designed for execution within a browser environment.

Actually Capi is 100% designed for the browser environment as we are using Deno which has full support for web standard APIs. There are just hiccups along the way when going from Deno -> Node.

Could you also clarify if Capi is supporting the Node 20 version, or specifically the 20.3 version as described in a https://github.com/paritytech/capi/issues/1054#issuecomment-1588754090?

Any Node 20.x should work.

tjjfvi commented 1 year ago

Any Node 20.x should work.

@ryanleecode This is not true – earlier versions of Node 20 had a critical bug that causes ts-node to fork bomb.

tjjfvi commented 1 year ago

There appears to be an implication that Capi may not be designed for execution within a browser environment. I would be grateful if you could verify this presumption and, if it is indeed the case, elucidate the preferred environment for Capi.

We support the browser, and we prioritize it over Node. We should & will improve that wording (#1163).

Furthermore, I would appreciate some clarification on the term "for the sake of browser compatibility" mentioned in the description.

We prioritize the browser, and thus we use the native Web Crypto API, which is supported by Deno, Node, and browsers.

The Web Crypto API is usually accessible from globalThis.crypto, and this is what we access in Capi. However, on Node 18 (and below), Node's implementation of the Web Crypto API is only accessible from require('node:crypto').webcrypto.

Thus, to support Node 18, instead of accessing globalThis.crypto, we would need to import node:crypto, which would require those using Capi in the browser to use a shim for that package.

One can (to my knowledge) use Capi with a lower version of Node by adding a shim for globalThis.crypto:

import { webcrypto } from "node:crypto"
globalThis.crypto = webcrypto

(this should & will be documented (#1163))

Thus, we have a tradeoff; we must either:

Between the two options, I think the latter is preferable. The browser is a more important use case for us, and the Node shim isn't necessary as Node 20 is adopted (it will be LTS in October).

rflechtner commented 1 year ago

Somehow I have missed this version constraint when trying to figure out why my code was blowing up on the most recent LTS release of node. Have you considered documenting node version compatibility in the package.json engines field?

vjjft commented 1 year ago

@rflechtner That should happen. (added #1178)

The shim for compatibility with <20 is being documented (paritytech/docs.capi.dev#40).

If either/both of the capi package postinstall or the capi cli would check the node version and facilitate any necessary shim, that would be good imo. Thoughts?

cc @harrysolovay

harrysolovay commented 1 year ago

Developers may have opinions about how they shim the Web Crypto API. I suppose the CLI could prompt the developer and give them several choices, but this seems out of scope for now. Imo best path forward is engines field + getting those docs merged.

vjjft commented 1 year ago

path forward is engines field

afaict that will result in a warning to all <20.3.1 with no context or indication that it actually is compatible

harrysolovay commented 1 year ago

Good point. Should we have the Capi CLI produce a custom warning?

vjjft commented 1 year ago

I have created issue:

to facilitate followup on that topic.

afaict, the original topic of this issue has been addressed, and the related issues are now tracked separately, so imo this issue could be closed