Skip to content
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

RFC: Separate SDK into multiple packages by platform #309

Closed
shamilovtim opened this issue Apr 11, 2023 · 4 comments
Closed

RFC: Separate SDK into multiple packages by platform #309

shamilovtim opened this issue Apr 11, 2023 · 4 comments

Comments

@shamilovtim
Copy link
Member

shamilovtim commented Apr 11, 2023

Context

Looks like sooner than later we’re going to need to split up the DWN SDK into browser, node, and maybe react native packages. They seem to conflict with each other, with different bundling / module resolution, dependencies, and so on.

Most recently bumped into the fact that DWN SDK tries to use classic-level and classic-level is Node only. It pulls in a bunch of deps like os, fs and so on that I needed to polyfill. I can only polyfill so many until it gets too crazy.

I think this is probably because the package tries to be zero configuration and very user friendly. This ends up shooting you in the foot if you’re on neither browser or a node.js server (e.g. react native).

The sheer amount of Node.js libs being included would make me nervous about whether tree-shaking can even deal with everything in a bundled browser environment

I think by separating into node, react native, and browser SDKs you can have fine grained control over what’s included, an eye on the bundle size and the transitive dependencies and more easily start slimming things down and keeping deps up to date

Right now transitive dependency resolution feels complicated. I have multiple versions of the transitive dependencies of the DWN sdk in my project. I think because it’s so liberal about how many dependencies and their versions it brings in, and how many platforms it’s supporting.

Proposal

  1. Deprecate dwn-sdk-js
  2. Create some combination of the following (monorepo optional):
  • @tbd54566975/dwn-core
  • @tbd54566975/dwn-node
  • @tbd54566975/dwn-browser
  • @tbd54566975/dwn-react-native
@shamilovtim shamilovtim changed the title RFC: Separate SDK into multiple packages RFC: Separate SDK into multiple packages by platform Apr 11, 2023
@simonwh
Copy link

simonwh commented Apr 12, 2023

We have the same experience integrating the SDK in react native. It's doable, but requires a lot of polyfills and workarounds :-)

I like this approach, it follows the structure of many multi-environment projects out there!

@pax-k
Copy link

pax-k commented May 30, 2023

I wrote a script that might help you isolate node specific deps. It's a starting point, other checks can be useful like grepping the sources for fs for example. lodash should be Node.js and Browser but its package.json doesn't say too much for the checks I implemented. This is the result for dwn-sdk-js:

Browser only: [ 'ipfs-unixfs-exporter', 'ipfs-unixfs-importer', 'multiformats' ]
Node.js and Browser: [
  '@js-temporal/polyfill',
  '@noble/ed25519',
  '@noble/secp256k1',
  '@scure/base',
  '@swc/helpers',
  'abstract-level',
  'cross-fetch',
  'date-fns',
  'eccrypto',
  'level',
  'randombytes',
  'readable-stream',
  'secp256k1',
  'ulid',
  'uuid'
]
Node.js only: [
  '@ipld/dag-cbor',
  'ajv',
  'blockstore-core',
  'flat',
  'interface-blockstore',
  'interface-store',
  'lodash',
  'lru-cache',
  'ms',
  'varint'
]

Apart from browser compatibility, not all browser features are available in Hermes / react-native, like TextEncoder (can be polyfilled with "text-encoding" for example) or IndexedDB.

Worth looking into this also https://github.com/expo/browser-polyfill

@shamilovtim
Copy link
Member Author

@pax-k some of my comments here are a little stale actually. we've got a lot of great polyfills setup which are of higher quality than some of the deprecated older pure JS ones. We plan to follow up on this soon.

@thehenrytsai
Copy link
Member

Progress has been made with regards to getting the ESM and CJS packages to work in various environments. We are likely going to continue to build just these two types of packages, with the possibility of even removing CJS package now that Electron officially added support for ESM: electron/electron#37535

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants