foliojs / brotli.js

A JavaScript port of the Brotli compression algorithm, as used in WOFF2
494 stars 51 forks source link

Update brotli version and add dictionary compression encoding #51

Closed yoavweiss closed 6 months ago

yoavweiss commented 6 months ago

This PR updates the brotli code to the latest version, updates the Makefile to be able to build that, and moves to use brotli's streaming interface, instead of the older encode() interface.

In order to do that latter, I've added compress.c, which calls that streaming interface, and which includes dictionary as its inputs. When a dictionary is provided, it's attached to the Brotli state and used as part of the compression.

This PR does not add the equivalent dictionary capabilities to the decoder. A reasonable followup may be to call the WASM decoder for that purpose.

devongovett commented 6 months ago

Thanks!

devongovett commented 6 months ago

one problem with the change to compile to wasm is that bundlers have a hard time picking up where the wasm file is located. emscripten doesn't produce very statically analyzable output. could change emscripten to output ES modules so it can use import.meta.url but that might be a breaking change (it makes everything async). will need to consider what to do here before publishing.

yoavweiss commented 6 months ago

one problem with the change to compile to wasm is that bundlers have a hard time picking up where the wasm file is located.

Apologies, but I'm not sure I get the implications of that :) Is this something that my change introduced?

surma commented 6 months ago

I think switching to ESM output is generally a good idea. You could (should?) expose an option where you can pass the path/URL to the wasm blob, which usually is enough to let developers use a wasm-based library with any bundler tool.

jakearchibald commented 6 months ago

You could (should?) expose an option where you can pass the path/URL to the wasm blob, which usually is enough to let developers use a wasm-based library with any bundler tool.

Coincidentally, I was playing with https://github.com/httptoolkit/brotli-wasm this morning and the lack of this was a blocker when using a Vite project.

Here's their entry point: https://www.npmjs.com/package/brotli-wasm?activeTab=code

import init, * as brotliWasm from "./pkg.web/brotli_wasm";
export default init().then(() => brotliWasm);

Annoyingly, the init function (generated by wasm-bingen) does accept a URL. I patched it so the entry point was:

import wasmURL from './pkg.web/brotli_wasm_bg.wasm';
import init, { compress } from './pkg.web/brotli_wasm';
await init(wasmURL);
export { compress };

And that worked great.

So, yeah, +1 to allowing the wasm URL to be passed in, and ensure your wasm is exported in package.json.

devongovett commented 6 months ago

Is this something that my change introduced?

Yeah but only due to the upgrade of Emscripten. The previous one was so old it was still generating asm.js. That meant it was a single JS file. Now it needs to reference a WASM file as well.

Moving to ESM and making the WASM loading async would need to be done in a major version bump.