Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memorize camel-snake-kebab.core #17292

Closed
flexsurfer opened this issue Sep 15, 2023 · 6 comments
Closed

memorize camel-snake-kebab.core #17292

flexsurfer opened this issue Sep 15, 2023 · 6 comments

Comments

@flexsurfer
Copy link
Member

camel-snake-kebab.core is slow, if we use it it should be memorized. in worklets for example

@flexsurfer
Copy link
Member Author

cc @Parveshdhull

@Parveshdhull
Copy link
Member

Hi @flexsurfer,
Thank you very much for finding the issue and pinging.
Looks like most of the worklets functions are converting the kebab case to the camel case in the js code itself
(changed in #14855),
the only exception is the floating-screen-gesture-on-end function. Will fix this one too. Thank you

@flexsurfer
Copy link
Member Author

so we could remove camel-snake-kebab dep ?

@Parveshdhull
Copy link
Member

Parveshdhull commented Sep 18, 2023

Current use case of camel-snake-kebab.core in code
(if we can get rid of uses in these 4 instances, we can remove library)

  1. ns status-im2.contexts.quo-preview.preview fn humanize
  2. ns test-helpers.component
  3. ns utils.worklets.shell fn floating-screen-gesture-on-end
  4. event: :chat.ui/cache-link-preview-data

1st and 2nd should not affect app performance as they are just quo-preview and test-helpers
3rd is very easy to remove, just have to move conversion to js
4th not sure, why we need and how hard it will be replace, need to check

@ilmotta
Copy link
Contributor

ilmotta commented Oct 14, 2024

Is this issue still relevant @flexsurfer @Parveshdhull ?

@ilmotta
Copy link
Contributor

ilmotta commented Nov 8, 2024

@Parveshdhull, since the time of your comment a few things changed for the better cc @flexsurfer @ulisesmac:

  • CCs in the wallet team implemented memoization on csk/->kebab-case-keyword and csk/->PascalCaseKeyword. We can use these utilities when appropriate.

(def ->kebab-case-keyword (memoize csk/->kebab-case-keyword))

As Parvesh said, the non-prod code using csk is harmless.

Then we have this other point:

ns utils.worklets.shell fn floating-screen-gesture-on-end

This is also now only using clj->js, not csk anymore.


So this issue can be closed and we don't need to remove the library if we use it with memoization (I'd still avoid it if possible, but it's more usable with memoization).

Shall we close as completed @flexsurfer ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants