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

Equivalent of @solid-primitives/refs mergeRefs but for event listeners #546

Open
tronikelis opened this issue Nov 25, 2023 · 6 comments
Open

Comments

@tronikelis
Copy link

Describe The Problem To Be Solved

Hi, so when I am creating components almost always I need to pass some callbacks to onClick or anything else like that, but I want the user of the component to be able to pass onClick as well, so now I need to pass that event to splitProps, and don't forget to pass both onClick and onclick because both variants could be used by the component user and then finally check if it is a function and then call it with the same params.

Doing this everytime is a burden.

Suggest A Solution

How I think about solving the problem:

  • the helper should integrate the splitProps function from solid, as you would always use splitProps to get the callbacks
  • the helper should call both the camelcase and the lowercase versions of the event, so if I use onClick inside the component, it shouldnt matter if the user passes onClick or onclick to the component, both of these should get triggered
  • fully type-safe, but I assume this is the default for all of the helper fns

My very rough first try at implementing this:

import { mergeProps, splitProps } from "solid-js";

type AnyFn = (...params: any) => any;

type ExtractFunction<T> = Extract<T, AnyFn>;

type WithCallback<T> = {
    [K in keyof T as ExtractFunction<T[K]> extends never ? never : K]?: ExtractFunction<T[K]>;
};

export default function splitPropsWithCallback<
    T extends Record<any, any>,
    K extends (keyof T)[],
    G extends WithCallback<T>,
>(props: T, split: K, callbacks: G) {
    const [local, others] = splitProps(props, split);

    const withCallback = () => {
        const keys = Object.keys(callbacks) as (keyof typeof callbacks)[];
        const final = {} as typeof callbacks;

        for (const key of keys) {
            const inner = callbacks[key];

            const combined = (...params: any[]) => {
                if (typeof props[key] === "function") {
                    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
                    props[key](...params);
                }
                if (typeof props[key.toString().toLowerCase()] === "function") {
                    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
                    props[key.toString().toLowerCase()](...params);
                }

                // eslint-disable-next-line @typescript-eslint/no-unsafe-call
                return inner?.(...params);
            };

            // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
            final[key] = combined as any;
        }

        return final;
    };

    const mergedOther = mergeProps(others, withCallback);

    return [local, mergedOther] as const;
}

usage: https://playground.solidjs.com/anonymous/96db2b0c-0846-47ed-86ef-0e275c4b24e7

@gabrielmfern
Copy link

love this idea +1, for one of my project I had to make a hacky mergeCallbacks for this sorta of stuff

I wonder if it makes sense that this goes into @solid-primitives/props as well, like the combineStyle

@tronikelis
Copy link
Author

what I am using right now is mergeRefs + createEventListener:

    const onInput = () => {
        if (local.value && context) {
            context.setSelected(local.value);
        }
    };
    createEventListener(() => ref, "input", onInput);

This way I don't worry about the consumer overriding my events, seems like a better solution than hacking around with very custom stuff

@thetarnav
Copy link
Member

thetarnav commented Feb 18, 2024

+1 to just adding a bit of custom logic when needed

if ("onClick" in props) props.onClick(e)
localOnClick(e)

And for doing that automatically for all props, combineProps should do the job. It handles combining lowercase/camelCase handlers as well.

But I like the idea of the mergeCallbacks helper.
I think you could do the same with chain helper from utils

onClick={chain(
	e => console.log("click:", e),
	props.onClick,
)}

@tronikelis
Copy link
Author

@thetarnav oh wow, did not know about combineProps and chain, very useful

@gabrielmfern
Copy link

+1 to just adding a bit of custom logic when needed

I'd agree it's easier, but the problem with that is that you will have to override the handler types from ComponentProps if you do this, because its types allow for bound event handlers. Does chain handle those as well?

@thetarnav
Copy link
Member

Good point @gabrielmfern, it does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants