httptoolkit / brotli-wasm

A reliable compressor and decompressor for Brotli, supporting node & browsers via wasm
Apache License 2.0
264 stars 21 forks source link

Support Node.js ESM #39

Open kettanaito opened 1 month ago

kettanaito commented 1 month ago
CLAassistant commented 1 month ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

kettanaito commented 1 month ago

Hi, @pimterry 👋 Can you please help me with the module tests for this change? I'm not entire familiar with what you are using for that. Thanks!

pimterry commented 1 month ago

Can you please help me with the module tests for this change? I'm not entire familiar with what you are using for that. Thanks!

There's no formal framework. There's a single file of Mocha tests in test/, written in TypeScript, and there's scripts and to launch that same test file in various different ways for each env in package.json.

We'd need a new script there, which runs those tests in Node via ESM.

I'm not sure what the best approach to do that is - the only constraint is sharing the same test code (I'd really like to avoid duplicating this) but if you want to make separate Node vs Node-ESM entrypoints then that would be fine. Changes to the cross-compat code at the start of the test file (or even splitting it from the test implementations) is fine too, up to you.

Since this isn't usable in old Node anyway, you could skip these tests in that case, and in that case you could also potentially use --experimental-strip-types to handle the TypeScript part.