benjamn / wryware

A collection of packages that are probably a little too clever. Use at your own wrisk.
MIT License
87 stars 15 forks source link

Proposal: record-tuple-polyfill #120

Open mister-what opened 3 years ago

mister-what commented 3 years ago

Babel recently introduced a support for the Record and Tuple ECMAScript proposal. Babel currently uses @bloomberg/record-tuple-polyfill as the underling implementation. But I was wondering how this compares to @wry/record and @wry/tuple and wether they would be usable as an alternative polyfill?

I'm favoring @wry/record and @wry/tuple from an implementation perspective. Correct me if I'm wrong, but @bloomberg/record-tuple-polyfill seems to be unnecessary complex and convoluted in comparison. I'd like to propose a new package @wry/record-tuple-polyfill if semantic-, API- and formal compatibility are given.

(Non exhaustive) list of requirements for this proposed package:

  1. re-exports of Tuple from @wry/tuple and Record from @wry/record
  2. additional tests to ensure formal, semantic and API compatibility with @bloomberg/record-tuple-polyfill
  3. (stretch goal or pre-condition?) a benchmark suite (that also covers memory consumption)

Requirement 1. should be trivial to do. It will be something like this:

export Record from "@wry/record";
export Tuple from "@wry/tuple";

Meeting the second requirement might be a little bit more tricky. I think a property based testing approach could give the strongest guarantees here.

Not sure if the benchmark suite should be rather a precondition for publishing the new package or if is a rather a stretch goal.

(any input/discussion is very appreciated)

benjamn commented 3 years ago

@mister-what Thanks for bringing this to my attention!

I haven't had a chance to dig into the details of the @bloomberg/record-tuple-polyfill implementation, but I'm inclined to agree that relying on WeakRef and FinalizationRegistry as a hard requirement seems undesirable, since those recently-standardized features fundamentally cannot be polyfilled (because the ability to observe garbage collection is a truly new language feature, not just a convenience for something that was already possible).

If I had to speculate about the difference between the two approaches, the @bloomberg/record-tuple-polyfill package is probably able to unwind/discard more of its internal lookup graph when object references in tuples or records get garbage collected. That's what I would do if I thought I could get away with depending on FinalizationRegistry, at least.

In a WeakMap-only implementation like that of @wry/trie, whole subtrees can be garbage collected when their root object becomes unreachable (thanks to the weakness of the WeakMap), but there's no way to listen for that silent deletion of keys from the WeakMap (without FinalizationRegistry), so it's harder to detect when it might be possible to delete Trie nodes with no assigned data and no remaining children. On the other hand, those Trie nodes might be used again in the future, since the sequences of keys to access them are still in memory (perhaps permanently, because the keys are primitive), so deleting those nodes is not an absolute imperative.

In other words, if all you have is WeakMap, you can tell a pretty reasonable garbage collection story, but you need FinalizationRegistry to do a perfect job of eagerly cleaning up the internal lookup graph.

Finally, I can't make any promises that these @wry/record and @wry/tuple packages match the Record and Tuple spec proposal. For example, equality of tuple elements in @wry/tuple essentially boils down to Map key equality, whereas I seem to remember there was some discussion by TC39 of making sure that === values like -0 and 0 are preserved by Tuples (kept in separate Tuples). If that's still a goal of the proposal, it's definitely not a goal of these packages, and it might be a contributing factor to the complexity of @bloomberg/record-tuple-polyfill.

With all of that said, I'm open to pursuing your idea of a @wry/record-tuple-polyfill further, especially if we can find a comprehensive test suite somewhere…

mister-what commented 3 years ago

Hi @benjamn, thx for the comprehensive and exhaustive answer. Sorry that I was not able to answer earlier. The Readme of tc39/proposal-record-tuple provides a sane set of properties for records and tuples that must hold. Additionally, there is the spec. Admittiatelly, my brain is only capable of extracting limited information that could be used to define a comprehensive test suite.

But: the Bloomberg polyfills already have a test-suite that might provide a good baseline. Writing a generic test suite for any Record or Tuple implementation/polyfill should be very feasible. Nevertheless, the TC39 Process requires tests for new feature proposals in the entrance criteria for Stage 4. So we have two options here:

  1. Waiting for a tc39/test262 test-suite of the proposal.
  2. Implementing a generic test-suite (maybe even in a test262 compatible way) and contributing this back to the ECMAScript proposal.