search

mtth / avsc

Avro for JavaScript :zap:
MIT License
1.27k stars 147 forks source link

Refactor import hook API to subsume all platform-dependent behavior #445

Closed valadaptive closed 6 months ago

valadaptive commented 8 months ago

First off, apologies for leaving you hanging for a year! Some stuff came up and this fell off my radar for a while, and this seemed like it was going to be more fiddly and difficult than it ended up being.

This import hook API isolates all platform-specific path resolution stuff within the import hook itself. The hook now takes the path of the imported file and the path of the file that imported it, and calls the callback with the file contents, and the resolved path of the imported file. That second part is what stumped me on my last attempt.

I also started work last year on a "de-Buffer-ification" branch, but ran into some performance cliffs. I finally figured those out (except for TextEncoder/TextDecoder, which is still slower in Node sadly), and the code is now on GitHub. It's actually faster on most benchmarks than the Buffer version. It's still a proof-of-concept and will need to be cleaned up. If you're interested in merging it before the major version bump, maybe remove services first, so I don't have to port that over to use Uint8Array.

mtth commented 7 months ago

No worries and thanks for the PR @valadaptive! This change looks good overall, will send a review shortly.

Great to hear about the Uint8Array-related changes. I would probably release those with the upcoming major version. Could you share performance numbers?

mtth commented 7 months ago

Thanks @valadaptive. The numbers above look great. Did you mean to push changes to this branch?

valadaptive commented 7 months ago

Yup, my bad! Changes have been properly pushed now.

valadaptive commented 6 months ago

I've made those last few changes and also stopped exporting existsSync and readFileSync now that that functionality's been moved into files.js.