Replies: 7 comments 1 reply
-
Perhaps Chevrotain should use a 3rd party utils library again?It is also possible to challenge my past decision to implement the custom functions manually. Things to consider:
|
Beta Was this translation helpful? Give feedback.
-
I don't think mixing styles is much of an issue. It makes clear which are standard functions and which not. The main advantage to me for replacing custom functions with built in ones is that it makes it easier to read the code for everyone as most JS developers are going to be familiar with the standard functions. As for custom-library/not very rich standard lib, almost all the custom functions are covered by the built ins nowadays. E.g. flatten can be replaced by flatMap in most uses in the codebase. EDIT: Oh yeah and as for null-safety/undefines, I wanted to enable strict in the tsconfig anyway, this should solve that issue. |
Beta Was this translation helpful? Give feedback.
-
I'm thinking of trying to use a 3rd party utils library again and completely remove the @chevrotain/utils pakage, so read my reply with that context in mind.
I agree it can make the code easier to read for some (many?), although it can make the code harder to write / maintain e.g:
I'm not sure sure all can be replaced directly, e.g:
and I'd prefer to write logic in a common generic functional style when possible.
|
Beta Was this translation helpful? Give feedback.
-
My primary objective is to reduce the TCO of developing / maintaining Chevrotain. This would also support making the code base more standardized if a popular 3rd party library is chosen.
This would also remove any concerns in regards to runtime engine compatibility as long as the 3rd party library chosen targets a lower/equal ECMAScript version than Chevrotain (another reason to stop targeting ES5.1...) The main disadvantage would be the increased bundle size, but perhaps this can be mitigated nowadays by modern bundling tools |
Beta Was this translation helpful? Give feedback.
-
This is a good example for the compatibility issue with the native built-in functions. I randomly spotted this while trying to decide which runtime to target in #1272 ... |
Beta Was this translation helpful? Give feedback.
-
Very handy.
I'll probably would like to make an attempt first to replace everything with a 3rd party library as I would like to keep a consistent style It is obviously a subjective opinion, but an hybrid approach would not allow me to get rid of (virtually) all of the utilities package
To be honest I would have liked to play with something that is supposedly more pure functional style like Ramda.
|
Beta Was this translation helpful? Give feedback.
-
Note that introducing a 3rd party lib would likely increase the minified package size for some consumers (web based). |
Beta Was this translation helpful? Give feedback.
-
This discussion originated here: #1679
Background
Early on Chevrotain used 3rd party utilities library such as lodash/underscore,
But at some point I wanted to remove external dependencies, so I've implemented the basic high order functions as needed.
Different Styles
Nowadays the standard library is richer, However it is still not very rich... so this creates two style of invoking high order functions.
arr.foo(() => {})
foo(arr, () => {})
The first:
uses the standard library so one could claim it is more idiomatic.
Requires validating if it is supported in the targeted runtimes
Note the targeted runtimes and ECAMScript spec are both "moving targets".
Would throw exception at runtime if arr is undefined.
Which means we need to be certain the arr can never be undefined in any
The second may be less idiomatic, but also more stable, both in terms of tracking runtime capabilities and possible runtime exceptions.
Consistent Style
But there is another concern, which is consistent style.
The two styles above read very differently, I would rather not mix and match those two styles, particularly not in the same function/file/project. So until some point in the future where ECMAScript has a proper rich standard library which is supported in all runtimes, maybe it is better to stick with custom functions.
Beta Was this translation helpful? Give feedback.
All reactions