Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropping support for old Node versions + improving browser support? #418

Open
valadaptive opened this issue Nov 30, 2022 · 5 comments
Open

Comments

@valadaptive
Copy link
Contributor

Since the days of Node 0.x, a lot of things have changed in the Node ecosystem.

When Node was first created, a lot of useful functionality was missing from the ECMAScript standard, so Node created its own set of APIs which were polyfilled by e.g. Browserify.

Over time, APIs covering the same or similar ground have been standardized to work in both Node and the browser, and browser-based polyfills for Node APIs have lost much of their utility--many aren't even being maintained anymore. Browserify has given way to bundlers like Webpack and Rollup which focus less on emulating Node in the browser and more on simply packaging all the code together.

I ran into this when trying to use avsc in my web application--the newest release of Webpack doesn't automatically polyfill Node features anymore, and I had to figure out which polyfills avsc did and did not require to function, then add each one to the Webpack configuration myself.

By dropping support for old Node versions (0.12 reached end-of-life at the end of 2016), a lot of Node-specific dependencies could be avoided, making the project easier to use in the browser. For example, avsc makes heavy use of util.inherits, which could be replaced by ES6 classes. See also #410, which proposes replacing Buffer with the browser-compatible Uint8Array.

Do you have any interest in bumping the minimum Node version (not even to the latest version--just to one which supports ES6), migrating the codebase to ES6, and reducing Node-specific dependencies? I'd be more than happy to do it myself, but I don't want to make massive unsolicited PRs without knowing whether they'd actually be helpful.

@mtth
Copy link
Owner

mtth commented Dec 20, 2022

Hi @valadaptive. Thanks for bringing this up and offering to help. A PR which migrates the codebase to ES6 and reduces Node-specific dependencies would be welcome. I'd be happy to review it and release a new major version from it. We'll also want to confirm there are no significant performance regressions, for example <5% slowdown on the benchmark schemas.

@valadaptive
Copy link
Contributor Author

With #481 merged in, the only polyfill needed to run avsc in the browser is stream. Right now, require('avsc/etc/browser/avsc-types') doesn't seem to work (at least in Webpack).

It may be possible to redo package.json to use conditional exports, which should allow for the proper export to be used for the target environment (Node or browsers) without having to alter the import path. If so, which paths should be explicitly exported?

Regarding streams more broadly, I could probably rewrite containers to not use the stream API explicitly, and expose browser-specific functionality like createBlobDecoder and createBlobEncoder using the new web streams API. Is such a thing desirable as well?

@mtth
Copy link
Owner

mtth commented Sep 28, 2024

As part of the TypeScript migration, I was thinking of restructuring the repo into multiple packages. For example:

  • @avro/types, just types. Usable without changes both in Node and the browser.
  • @avro/node-containers, Node-stream-based containers. Only usable in Node.
  • avsc, which aggregates and re-exports all other packages as a convenience.

In the future, we could add other packages like @avro/web-containers for Web-based containers. Alternatively, we could have a single @avro/containers with conditional exports but that will require updating the API of both packages to match.

WDYT @valadaptive?

@valadaptive
Copy link
Contributor Author

That sounds like a good idea. Once you get TypeScript set up here, I can look into setting up some sort of monorepo if you want. I prefer the idea of separate @avro/node-containers and @avro/web-containers packages.

NB: you may want to choose a different name than @avro/types, since there are @types/[package name] packages for TypeScript definitions (and a "types" field in package.json for the same purpose), and people might assume @avro/types is TypeScript-specific.

@mtth
Copy link
Owner

mtth commented Sep 29, 2024

I prefer the idea of separate @avro/node-containers and @avro/web-containers packages.

SGTM.

people might assume @avro/types is TypeScript-specific.

Hmm. I see how @avro/types can be confusing. Maybe @avro/values would be better, though its main exported symbols are (Avro) types and called Type, IntType, etc.

Thank you for the monorepo setup offer. Switching to pnpm will be part of the change already :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants