Replace lodash with native ES2015 functions

[joliss]

Fixes #1870.

This patch brings packages/chevrotain/lib/chevrotain.min.js from 201 KB down to 158 KB.

It's split into two commits for better reviewability: one to use native ES2015 functions instead of lodash, and one to reformat everything with prettier (which causes a lot of lines to be reindented) and run pnpm install.

I tried to keep track of what features I'm using to replace lodash:

• ES2015:

  • Array.prototype: reduce, map, filter, forEach, every, some, slice, find
  • Array.isArray
  • Object.keys
  • Object.assign
  • Object.prototype.hasOwnProperty
  • Set

• ES2018:

  • spread in object literals { ...options, foo: true }; I'm using this and relying on the compiler to make it ES2015-compatible

• ES2020:

  • null-ish coalescing operator ??; relying on the compiler for this as well -- I verified that both ?? and ... are indeed compiled away in packages/chevrotain/lib/chevroain.js

The following features I'm not using, to preserve ES2015 compatibility. They might be worth revisiting in the future if we bump the minimum ES version:

• ES2016:

  • includes: using indexOf instead

• ES2019:

  • flat: using ([] as SomeType[]).concat(...arr) instead
  • flatMap: using map followed by ([] as SomeType[]).concat(...arr), or a helper function (see comment)

• not (yet?) in ES:

  • groupBy: using a three-liner instead

I'll leave a few comments on the diff right after posting this PR, to point out a few things you might want to check are OK before merging this. If there's anything you think needs to be changed, just let me know and I'll try to update the PR; or feel free to amend the commit as you see fit.

[joliss]

I'll leave a few comments on the diff right after posting this PR

Ok, all done. I think it's ready for review!

[mattbishop]

That's a lot of work, thanks!

Can you run web_benchmark and see if there's a perf difference?

[bd82]

I'm leaning towards not accepting this PR.

a ~20% reduction in bundle size simply does not seem to be worth (for me) the:

• Reduction is code readability (less declarative code).
• Added complexity of checking / validating availability of built-in functions in future features.
• Poorer set of available utilities for future features.

Maybe this is because my use cases for Chevrotain are less sensitive in regards to bundle size... Or maybe because the amount of time can dedicate to maintain Chevrotain is smaller than in the past so I weigh maintainability related features higher...

[mattbishop]

It might be early, true.

How about performance? @joliss csn your I run the benchmark suite?

[bradenmacdonald]

Wow, awesome work! I was reviewing my project's dependencies today and annoyed by the 235 (!) lodash source files that are in the deno.lock dependency file (pre-bundling) because of Chevrotain.

I haven't yet contributed to Chevrotain's codebase myself, but as someone who works with modern TypeScript code all the time, I find the version in this PR much easier to read and follow because it uses standard functions and syntax - the same ones I'm used to. I am not familiar with assign, identity, compact, drop, flatMap, values, uniq, noop, etc. Even though those functions may follow common conventions, they're still an extra layer of indirection that I would need to learn and think about compared to built-in things. So I want to point out that I would disagree with regards to code readability - I think this change generally makes the code more readable (except in a few small instances), and thus has the benefit of making the codebase more accessible to potential contributors. However, it is a stylistic difference with the key function name (like map etc.) being moved to the right of the value, not preceding it, so I understand why it may be less appealing.

[joliss]

@bd82 I wasn't able to figure out how to run the benchmark, and I don't have my development machine with me over the holidays. But if you're up for it, I was wondering you might be able to try and benchmark to compare this PR's commit with its parent, to see if it makes any difference?

[bd82]

I wasn't able to figure out how to run the benchmark, and I don't have my development machine with me over the holidays. But if you're up for it, I was wondering you might be able to try and benchmark to compare this PR's commit with its parent, to see if it makes any difference?

@joliss I will try to find some time to look into this again.

I doubt there will be a performance regression as the performance critical paths normally avoid the high order functions (e.g map / forEach / reduce / ...).

[bd82]

So I want to point out that I would disagree with regards to code readability - I think this change generally makes the code more readable (except in a few small instances), and thus has the benefit of making the codebase more accessible to potential contributors

@bradenmacdonald:

It is quite possible I have certain biases and gotten "used" to certain styles. I will have another look at the PR and re-evaluate it.