earthstar-project / earthstar

Storage for private, distributed, offline-first applications.
https://earthstar-project.org
GNU Lesser General Public License v3.0
639 stars 20 forks source link

detChoice problems in browser #48

Closed sgwilym closed 3 years ago

sgwilym commented 4 years ago

What's the problem you want solved?

This usage of detChoice:

detChoice(author.address, ["alpha", "beta", "gamma"]);

Results in the following error:

image

Am I using it wrong somehow?

And: is this function meant to be used in the browser? Calling it in a client gives me warning messages about how resource-hungry the current tab is.

cinnamon-bun commented 4 years ago

Thanks for the report!

That's the right usage (assuming author.address is a string).

https://github.com/earthstar-project/earthstar/blob/e0eb93063b9cf3c625e87be8af288d82faa1a3ef/src/util/detRandom.ts#L43-L45

https://github.com/earthstar-project/earthstar/blob/e0eb93063b9cf3c625e87be8af288d82faa1a3ef/src/util/detRandom.ts#L24-L33

...I haven't tested this in the browser yet. Since the error is coming from node_modules/buffer I'm guessing it's related to the browserification of node Buffers into browser ArrayBuffers. Maybe readUInt32BE isn't supported in the browser polyfill. I think sha256 is working so it's not that (if signing documents works, sha256 works).

How are you bundling this, is it browserify or webpack?

Can you get a more detailed traceback?

Speed

It should be very fast to run once, the problem is running it for every rendered React component over and over.

Faster hash function

We should switch from sha256 to md5 or something even faster. This isn't security critical.

Wait for sodium to load WASM

Investigate this note about waiting for sodium to load its WASM module. Is the slowness only temporary until WASM is loaded, or does it get stuck with the slow version if we use it without waiting?

Memoize

Memoizing / caching the function should fix it, but it will still be slow on the first render.

Something like:

let colorCache = {}  // mapping from author address to color string

let authorToColor(author: string): string => {
    if (colorCache[author]) { return colorCache[author]; }
    let color = detRandom(author, ['red', 'green', 'blue']);
    colorCache[author] = color;
    // todo: if cache is getting too big, randomly delete an item here
    return color;
}
cinnamon-bun commented 4 years ago

This reminds me about #2 Make Tests Run In Browsers ;)

If you have experience with that, I'd appreciate help.

cinnamon-bun commented 4 years ago

I verified that detChoice works in the browser when bundled using Browserify.

I'm guessing this error happened in earthstar-lobby which uses Webpack (via react-scripts). I think this is the webpack config, but it's kind of intimidating.

Theory

Maybe Webpack doesn't bundle earthstar correctly? This could cause a problem in 3 places

I'm not sure how everything could have worked so far but broken when detChoice appeared. I suspect it's this line:

https://github.com/earthstar-project/earthstar/blob/e0eb93063b9cf3c625e87be8af288d82faa1a3ef/src/util/detRandom.ts#L30

Webpack uses a pretty old polyfill, node-libs-browser, and I am guessing it doesn't understand readUInt32BE which might be new-ish.

If this theory is true, all of the earthstar code should work except any of the det functions (detChoice, detRandom, etc), which should all fail.

Other theory

Back over here in earthstar, Browserify is instructed to swap out some files when bundling. It uses this section of package.json:

  // we need to skip all the sqlite-related code when bundling for the browser
  "browser": {
    "./build/index.js": "./build/index.browser.js",   // swap out index for a version that doesn't import sqlite
    "./build/storeSqlite.js": false,   // skip the other file that imports sqlite
    "tap": "tape"  // use a browser-compatible test module
  },

I can't figure out if Webpack understands this browser field. I think it expects it to point to a single alternate index file, like this:

    "browser": "./build/index.browser.js",

???

cinnamon-bun commented 4 years ago

@sgwilym

I published earthstar 5.2.4 -- can you check if this fixes the problem?

4f9c89d8eff0b33da4cb84a04af84ce4df000a0c

I just copy-pasted node's code for buffer.readUInt32BE right into Earthstar, to escape from the clutches of a browser polyfill.

If it doesn't work can you point me to a commit in earthstar-lobby that triggers the error?

cinnamon-bun commented 3 years ago

Closing this for now since it seems to not be an issue lately. Please re-open if it can be reproduced.