mobily / ts-belt

🔧 Fast, modern, and practical utility library for FP in TypeScript.
https://mobily.github.io/ts-belt
MIT License
1.12k stars 31 forks source link

Questions about common FP functions #7

Closed anthony-khong closed 2 years ago

anthony-khong commented 2 years ago

Hi @mobily, the library looks really interesting! I'm quite interested in trying it out for future projects. I just have a couple of questions about certain functions, and I hope this is the right place to ask.

  1. How would you do Clojure's assoc and dissoc with ts-belt? Would it have to be D.merge(m, {k: v}) and D.rejectWithKey(m, (_, key) => key == k)?
  2. I noticed that D.xxxWithKey's signature is (v, k) instead of the usual (or rather what I would expect to be) (k, v). Is that just a matter of personal preference or rather a convention from another library/language that I'm not familiar with?
  3. On Remeda, the doc says "Due to the lazy evaluation, it’s also unclear how to use some utility functions within the pipeline." Does this mean that ts-belt does eager evaluation? So something like, say, pipe(m, D.toPairs, A.take(2)) would iterate through all the key-value pairs first before taking the first two elements?

Also, something like Remeda's mapping doc would be very helpful to newcomers! I'm happy to chip in if you think it'd be useful.

mobily commented 2 years ago

hey @anthony-khong 👋 thanks for your questions!

  1. that sounds like proper solutions for both, but first I need to look into it
  2. actually, you're right! it definitely should be (key, value), don't you mind creating a new PR?
  3. that's right, it does the eager evaluation, it iterates through all pairs and then takes the first two elements, and frankly speaking, I don't feel that the lazy evaluation is needed here, and it's not on the roadmap at this point (although, I'm open to suggestions)

Also, something like Remeda's mapping doc would be very helpful to newcomers! I'm happy to chip in if you think it'd be useful.

I'd be more than grateful if you could create something like this! ❤️

mobily commented 2 years ago

ad 2. it's been fixed and published in v3.1.1 🚀

anthony-khong commented 2 years ago

Hi @mobily, thank you very much for the response! And thank you for the patch to address (2)!

I'd be more than grateful if you could create something like this! ❤️

Absolutely, I've started looking into this. I realised though a lot of the functions I would love to use are not yet implemented, so I'm thinking of working on that first. If you're open to suggestions, would you accept contributions for Clojure's equivalent of assoc, assoc-in, dissoc, get-in, merge-with, select-keys, update and update-in? Of course, the function names can be adapted to the current function names (e.g. if we already have D.prop, it may make sense to have D.propIn instead of D.getIn).

If you agree to some of these proposals, I'd just like to make sure what it takes to implement these functions. I would have to implement the functions in src/Dict/Dict.res, declare the types in src/Dict/Dict.ts and write the tests in__tests__/Dict/xxx.test.ts, right? Am I missing any steps?

mobily commented 2 years ago

if we already have D.prop, it may make sense to have D.propIn instead of D.getIn

that’s actually a good point in terms of the naming convention, I took the prop name directly from ramda, however, in ReScript there are get (which returns Option) and getUnsafe and I would like to stick with this nomenclature, therefore, I've already deprecated prop and added get and getUnsafe, the changes will be released in the next version

the function names can be adapted to the current function names

the function names you posted look good to me. I'm only wondering if assoc and dissoc should be named set and deleteKey accordingly (same as can be found in ReScript: https://rescript-lang.org/docs/manual/latest/api/js/dict#set), but I don't have a strong opinion on this (we can create aliases for them eventually)

I would have to implement the functions in src/Dict/Dict.res, declare the types in src/Dict/Dict.ts and write the tests intests/Dict/xxx.test.ts, right?

yes, the only downside is that you need to rebuild the lib each time when you made changes (I need to rethink how to make this process more developer-friendly)

anthony-khong commented 2 years ago

the function names you posted look good to me. I'm only wondering if assoc and dissoc should be named set and deleteKey accordingly

I'm happy to change this on my latest PR (#8). Having a set of coherent function names is quite nice.

yes, the only downside is that you need to rebuild the lib each time when you made changes (I need to rethink how to make this process more developer-friendly)

The experience is okay for now. I can think of a couple of things that may be improved on top of my head:

I'm curious to know what you actually do during development!

mobily commented 2 years ago

👋 hey again @anthony-khong!

I'm curious to know what you actually do during development!

I did some changes to improve this process, so basically, for development purposes, you can use the following commands:

yarn build dev -n Dict -t set

⬆️ this command will build a single module (in this case Dict) and run set tests

yarn test run -f set -n Dict

⬆️ this command will be useful if you change a single test file (in this case Dict/set.test.ts)

anthony-khong commented 2 years ago

Hi @mobily, I'd just like to get your thoughts on something. I'm thinking about implementing setIn, getIn and updateIn. In particular, getIn(obj, ['a', 'b', 'c']) would be equivalent to obj.a.b.c. I believe the legit way of doing the types here is to go the lenses way, as discussed here. However, looking at how you did pipe.ts, perhaps something like this can work?

export declare function getIn<T, K0 extends keyof T>(keys: [K0]): (dict: T) => T[K0];
export declare function getIn<T, K0 extends keyof T, K1 extends keyof T[K0]>(
  key: [K0, K1]
): (dict: T) => T[K0][K1];
export declare function getIn<
  T,
  K0 extends keyof T,
  K1 extends keyof T[K0],
  K2 extends keyof T[K0][K1]
>(key: [K0, K1, K2]): (dict: T) => T[K0][K1][K2];
export declare function getIn<
  T,
  K0 extends keyof T,
  K1 extends keyof T[K0],
  K2 extends keyof T[K0][K1],
  K3 extends keyof T[K0][K1][K2]
>(key: [K0, K1, K2, K3]): (dict: T) => T[K0][K1][K2][K3];

Questions are:

mobily commented 2 years ago

Do you know of a better way of doing this?

this is related to #25, I will let you know once I figure it out

mobily commented 2 years ago

closing, updateIn/setIn/getIn functions have been added to my TODO list, but they have a minor priority at the moment :)