-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Magics: Introduce new $mixin
magic helper
#4200
Conversation
This will break getters and setters. To preserve them you would need more like components.reduce((acc, obj) => {
Object.defineProperties(acc, Object.getOwnPropertyDescriptors(obj));
return acc;
}, {}) instead of the spread operator. Spreading will simply take the value out of a getter right then, and setters would be lost entirely. I use the above (as a similar style of mixin) in production. Also, fyi, if you define it as an |
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.
Probably needs tests
Nice work, and good name for it. Can you give me some real-world examples to chew on before adding it? I'm leaning towards this being a good merge though. Also thanks for the notes @ekwoka |
Yep, you're right @ekwoka – completely forgot about the Will get that sorted and tested. |
Changes made r.e. |
@ryangjchandler I've triggered tests to re-run |
3dcb010
to
a6b09c0
Compare
Does anyone have any compelling real-world examples of this they'd like to share? or links to discussions or other community evidence around this? |
Few weeks ago i was doing an implementation for a wordpress premium form builder, had to create 3 custom form fields each of them are dependent on each other. The issue was i didn't had control over form element(tag) so i couldn't do: Mixing would solve this issue by mixing the 3 custom fields components |
Broadly, what this solves is just wanting to have the same root used for two (or more) components. This can be implemented with 2 elements with each of the components (increases div soup), or with multiple x-data on the single element (can provide questionable issues of the cascade in the event of overlapping keys). Those would be the two current ways to implement it without reaching for this kind of utility. It's overall simple to implement if a dev needs to, though the case of lifecycle methods and getters/setters may be missed or cause some headache. So it is another abstraction that does not NEED to be part of core, like some other functionality improvements might need to. I've only used it a very small number of times, and use it less and less with newer stuff (leaning towards custom directives for more and more stuff), but it is something that many would likely want to do and not fully understand how to. |
This pull request introduces a new
$mixin
magic helper that aims to make components more easily composable.At the minute, if you want to use multiple data providers for a component, you have to do something like this:
This doesn't feel very nice – plus it has some downsides such as
init()
anddestroy()
functions being overwritten by the final item in the spread.The goals of the
$mixin
helper are:init()
anddestroy()
functions correctly.The above code would become something like this:
If both
foo()
andbar()
have aninit()
,$mixin()
is smart enough to call both of those functions in the correct order (whichever order they're being mixed in). The same goes fordestroy()
methods.Of course, conflicting properties in each function are still an issue but that's not really avoidable without some weird object nesting stuff and I'd say that's not very intuitive.
Either way, I think this is a nice API for core and something I'd find myself using a lot, especially given the nicer syntax!
Let me know what you think 🤞