Allowing modern Javascript syntax (or specifying the earliest target Ecmascript version)

[mrflip]

The test suites -- or at least anything using npm -- are only runnable against 12.x. What do you consider the practical oldest-supported version of Javascript for this library?

tl;dr for the below -- would you accept a PR that changed the Cache (or other suites) to use classes, or that selectively used const/let or for (const item of items)?

--

I personally have very limited experience with non-modern Javascript, and it has served as a barrier to making contributions. The particular syntax in the current codebase that was difficult:

• Class rather than functional inheritance (rather than directly manipulating function's prototype)
• const and let give better correctness guarantees (there are edge cases of var that I don't understand, and I'm not aware of a drawbacks to mixing them.)
• Spread syntax ({ ...foo } and[...foo]) and for (const of iterable) { } are in most cases much clearer and let the VM make all the decisions (ie. my initial guess is that performance would be the same or better with the modern syntax).

Perusing https://node.green/ shows that cutting off at 10.x (ES2018) or 12.x (ES2019) gives nearly everything I'd call "modern".

Node 12.x: ES2019 and some of ES2020. Missing:

• ?.
• replaceAll
• ||=
• private class fields and methods
• .at()
• Error.cause

Node 10.x: ES2018 and some of ES2019. Notably missing syntax:

• Object.fromEntries
• Array.flat, flatMap
• Array.values() (which might be being used in the code?)
• Bigint missing from very earliest 10.x

I hope it's clear that I very much respect a decision to keep all versions of javascript alive as much as possible (and that I very very respect the clean, tested, performant, readable codebase).

[Yomguithereal]

@mrflip, until now the rationale was the following: I want to avoid any transpilation step for practical and performance reasons. For instance, TS and babel have a tendency to hit performance when transpiling because they overload code with some helpers to accomodate some edge cases of ES6 I don't care about. I can use the loose mode but even this does not cut it.

But since then, JavaScript changed quite a lot and transpilation is probably not required in most node/browser scenarios, so it might be time to upgrade the codebase. This said, this library is stilled used by a lot of legacy systems that don't have ES6 at all. But I am not against leaving them using older versions to advance.

tl;dr for the below -- would you accept a PR that changed the Cache (or other suites) to use classes, or that selectively used const/let or for (const item of items)?

Classes, why not. I want to avoid using for ... of internally because it is not performant enough (at least it still wasn't in node v18). But I make sure people can use it on the lib's structure if they want to.

I personally have very limited experience with non-modern Javascript, and it has served as a barrier to making contributions.

Ironically this was the contrary at the time this library started.

So, long story short: I don't have an issue with upgrading the code but I need it to still support most of real-life target (so the question is probably to cut to node 12 or 14). And I still need it to be consumable through require. Btw is the module schism over? Do we have a clear way to support all packaging system or is this still a ungodly mayhem?

[mrflip]

Agree that transpilation is not a good option for the codebase, unless it's specifically to serve the 10x and older crowd.

Classes, why not. I want to avoid using for ... of internally because it is not performant enough (at least it still wasn't in node v18). But I make sure people can use it on the lib's structure if they want to.

Yep I would not take anything farther away from the metal without having benchmarks in place first. Did not know that about the for...of.

the question is probably to cut to node 12 or 14

Node 12 has nearly everything you really need and there's probably a lot of AWS lambdas etc limping along on it. If this is mission-critical for a group on 10.x I think it's fair to ask for a PR that transpiles (without impacting modern users) and also gets the matrix tests to run and pass — or even to back out one of the few modernisms 10.x lacks; but with npm abandoning it there's no way to know what might break compatibility.

The module schism is.... about to be over? You can now require-ish things in the modern world, and dynamic import, but I still occasionally lose a programmer-day a month to some new weird piece of friction. No question that import is the future. I haven't had any issues using this from the other side:

import _                           /**/ from 'lodash'
import Mnemonist                        from 'mnemonist'
// ...
const { LRUMapWithDelete:Lru } = Mnemonist

---

This gives the guidance I needed, thanks.

[atombrenner]

I was wondering if we could start with modernizing the syntax in the documentation examples, e.g. using const and let instead of var. Unfortunately I cannot find the source for the documentation living on https://yomguithereal.github.io/mnemonist/heap. Any hint highly appreciated :)

[Yomguithereal]

@atombrenner the documentation lives in the gh-pages branch. I will of course review and accept a PR modernizing the variable bindings in the docs if you wish to open one :)

[atombrenner]

@Yomguithereal created PR https://github.com/Yomguithereal/mnemonist/pull/215