mcollina / async-cache-dedupe

Async cache with dedupe support
MIT License
640 stars 40 forks source link

Add browser support #48

Closed mateonunez closed 1 year ago

mateonunez commented 1 year ago

This PR would introduce the browser support for async-cache-dedupe as discussed in #47.

Context

This library is designed to run on Node.js. With support for Chrome, Firefox, Safari, and Edge., the async-cache-dedupe module can be extended between platforms and, in the near future, to other runtimes.

Test

I've added a test system to run all tests in the following browsers: Chrome, Safari, Edge, and Firefox. I also add support for the following bundlers: Esbuild, Webpack, Browserify, and Rollup.

To test correctly each bundler you need to run the following commands:

npm run test:prepare [esbuild|rollup|browserify|webpack]

This command will generate a JS file bundled and move it into a temporary folder.

To run the tests you should run the following command:

npm run test:browser [chrome|firefox|safari|edge]

This gonna run an index.html file that imports the generated suites containing tests. All the output will be streamed from the console of the browser to your Node console.

CI

The CI was added to run the browser tests in the following OS: ubuntu, windows, and macos. Each OS tests all the supported browsers with the supported bundlers. I've skipped the Safari tests on Ubuntu and Windows.

Caveats

There are some limitations to running this module on the client side.

mcollina commented 1 year ago

Good work! I think we might want to support Vite too.

I think we should not try to bundle https://github.com/mcollina/async-cache-dedupe/blob/main/src/storage/redis.js at all.

mateonunez commented 1 year ago

Hey @mcollina, thanks for the pre-review 😁

I've added Vite support as well and removed redis.js from bundles. The PR is still in draft because tests fail randomly. I need to investigate the error. I think it's because of timers, setImmediate, or now method. I'll let you know as soon as possible.

mateonunez commented 1 year ago

A lot of tests here are duplication of the Node.js ones. Could they be imported/loaded instead of copying them over?

Oh, I've tried that way... but I realized there are no shims for async_hooks (used by tap).

mcollina commented 1 year ago

let's see what @simone-sanfratello says!

simone-sanfratello commented 1 year ago

Nice work! Looks fine so far. Please remove all the // istanbul ignore and provide meaningful tests

simone-sanfratello commented 1 year ago

All good with the latest changes! :clap:

One last thing please: remove test:prepare script and run it in test:browser if needed

mateonunez commented 1 year ago

Hey, @simone-sanfratello. Thank you so much for the review and for your support. I've made all changes you requested.

simone-sanfratello commented 1 year ago

Well done!