fix: memoization when injecting magic

[AbhiShake1]

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)

[AbhiShake1]

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: Well actually, we dont need to explicitly cache at all

[ekwoka]

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 el and that can be passed it, so we don't need to create getUtilities over and over

[AbhiShake1]

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 el and that can be passed it, so we don't need to create getUtilities over and over

uhh i just overcomplicated it in my head. extracted

[calebporzio]

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?

[AbhiShake1]

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?

[ekwoka]

Looks good @calebporzio

@AbhiShake1 While some (like me) may disagree, Alpine is a prefer-let repo. Similarly, functions that are not actually used multiple places are normally kept in the same file as the code that it relates to. Here that would either be injectMagics or getElementBoundUtilities but not somewhere else. Thats the concept of "single file feature".

[AbhiShake1]

Looks good @calebporzio

@AbhiShake1 While some (like me) may disagree, Alpine is a prefer-let repo. Similarly, functions that are not actually used multiple places are normally kept in the same file as the code that it relates to. Here that would either be injectMagics or getElementBoundUtilities but not somewhere else. Thats the concept of "single file feature".

Thanks for explaining. Now it sounds right for this repo. Good to merge from my end

[calebporzio]

Thanks everyone!