earthstar-project / stone-soup

A specification and Javascript library for building online tools you can truly call your own.
https://earthstar-project.org
GNU Lesser General Public License v3.0
4 stars 2 forks source link

Package not usable everywhere due to import from util #38

Closed sgwilym closed 3 years ago

sgwilym commented 3 years ago

What's the problem you want solved?

This static import of util in src/util/bytes means that packagers like Rollup or Webpack try to import this package whether its imports will be used or not:

https://github.com/earthstar-project/stone-soup/blob/main/src/util/bytes.ts#L15

Here's an example error from Rollup:

Module 'undefined' has no exported member 'TextDecoder'. Did you mean to use 'import TextDecoder from 'undefined'' instead?

This means stone-soup doesn't work in a few different environments.

Is there a solution you'd like to recommend?

My first thought is to use esbuild's Inject API to insert the correct implementation of TextEncoder depending on whether the build is for node or the browser.

Then we just call TextEncoder and TextDecoder as though they are always correctly defined in src/util/bytes.

But there's a few problems:

cinnamon-bun commented 3 years ago

The esbuild inject API could certainly solve this. Maybe it's ok if it breaks the tests since we run the tests by building with tsc? But as usual I'm reluctant to have a hard reliance on esbuild for anything.

Another option is to make a platform.ts file in different flavors and have esbuild swap out the whole file. It would hold references or shims for everything platform-specific like TextEncoder, queueMicrotask, ... etc.

We'd have platform.node.ts, platform.browser.ts, platform.deno.ts.

The browser field in package.json can do this swapping. This is supported by esbuild, and browserify, but not sure about rollup and webpack:

  "browser": {
    "build/platform.js": "build/platform.browser.js"
  }

We already have files with those names, in the test folder, which I was using to list the drivers that are supported on each platform. We could move them out of the test folder and use them for this purpose as well?

We might also make a symlink from platform.ts to the appropriate file when we're developing, so that tsc knows which one to use when we try to import platform.ts, hmmmmm.

sgwilym commented 3 years ago

I share your reluctance to rely too much on a single tool. But doesn't the swappable platform file also mean a hard reliance on esbuild, and on a custom platform swapping plugin too?

The TextEncoder shim thing I can live with because it's a small plaster on our 'universal' code which isn't quite universal as we'd like it to be. And the really divergent stuff goes in the different entries and never gets mixed up.

sgwilym commented 3 years ago

Check out the PR above, it's quite compact!

cinnamon-bun commented 3 years ago

@sgwilym for comparison I tried out the platform.whatever.ts approach in this branch / PR: https://github.com/earthstar-project/stone-soup/pull/40

I don't think it has a hard reliance on esbuild because it uses the standard package.json browser field.

Not saying my approach is better for sure, just comparing the options :)