-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: memoization when injecting magic #4276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
It seems like getUtilities
can be moved entirely outside injectMagics
and just also accept the el
as an argument instead.
Thanks. I am not sure if the extracted function output should be cached where its cached now or maintain a global cache map. What would you recommend? Edit: |
Uh, I think it should probably be cached on the element itself...if we were to redesign it more. I jsut mean "getUtilities" as a function doesn't need to be scoped inside there. It it only references |
uhh i just overcomplicated it in my head. extracted |
I refactored this a bit to simplify (the method was extracted but not used elsewhere, so I brought it back in and also tweaked the code style to conform to the rest of the project). Are you both good with this before I merge? |
I had moved it to a different file since that function isnt a direct concern of magic. Thanks for tweaking the code style. Also, does using const instead of let violate coding standards of this repo in any way? |
Looks good @calebporzio @AbhiShake1 While some (like me) may disagree, Alpine is a |
Thanks for explaining. Now it sounds right for this repo. Good to merge from my end |
Thanks everyone! |
description
previously, the variable that was supposed to hold the value was being recreated for every loop. this has now been moved up to the hierarchy of dependencies. also, some minor edits have been made to make the code more readable and less error prone to further edits (subject to reviews)