-
Notifications
You must be signed in to change notification settings - Fork 2.7k
(Proposal) Rework the events API #10812
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
base: main
Are you sure you want to change the base?
Conversation
|
|
|
@gustavopch I agree, and I've updated the proposal to use |
|
Thanks for the thoughtful proposal, @aleclarson.
I don't believe this will be a big concern in practice since event names are easily prefixed (e.g. "rmx:press"). Is your main concern collision of event names?
Please say more about why this is a goal of yours. You dislike the syntax?
One important goal that we have is to avoid needing an // This sucks :/
import { on } from '@remix-run/interaction'
let button = <button on={[ on.click(...) ]}>Click Me</button>A separate |
I feel like having to import |
@afoures You're right, it does. I'm open to any ideas you may have about how to avoid it. |
The main concern, for me, is with code splitting. As I understand it, bundlers cannot "tree-shake" impure function calls.
It's a fine syntax, but it requires a Both of those concerns with the new I definitely understand the aversion to importing As far as having to import <div
on={{
click: {
capture: true,
listener(event) {…},
},
}}
>
{…}
</div>…or perhaps a more elegant solution? <div
on={{
click: {
capture(event) {…},
},
dblclick: {
once(event) {…},
},
}}
>
{…}
</div>…or you could flip it: <div
on={{
capture: {
click(event) {…},
},
once: {
dblclick(event) {…},
},
}}
>
{…}
</div>…and use |
<div
on={{
click: {
capture: true,
listener(event) {…},
},
}}
>
{…}
</div>I actually had the exact same thought this morning after I left my comment. We're already doing something similar in the router where you can define a route handler either as a plain handler function or as an object of As for code splitting concerns, we can address that easily through a separate module that contains our global registry map. This module would be a dependency of anyone who needs the registry. Unless I'm missing something, that shouldn't be an issue. |
|
another alternative could be to inline the factory inside the type Bind<target extends EventTarget> = <event extends EventType<target>>(
name: event,
listener: ListenerFor<target, event>,
options?: { capture?: boolean; once?: boolean; passive?: boolean },
) => Array<any>;
function on<target extends EventTarget>(
target: target,
cb: (bind: Bind<target>) => void,
options?: { signal?: AbortSignal },
) {
// ...
}
const button = document.createElement("button");
const press = defineInteraction("custom:press", () => {});
declare global {
interface HTMLElementEventMap {
[press]: PointerEvent;
}
}
const controller = new AbortController();
on(
button,
(bind) => [
bind("click", (event) => {}),
bind("click", (event) => {}, { capture: true }),
bind(press, (event) => {}, { once: true }),
bind("focus", (event) => {}, { passive: true }),
],
{ signal: controller.signal },
);even though the object approach resembles the way fetch-router routes work, i prefer this factory approach, for a few reasons:
but this approach would be a little unusual when writing JSX <div
on={(bind) => [
bind("click", () => { /* ... */ },{ capture: true }),
bind("focus", () => { /* not captured */ }),
]}
/> |
The issue appears when calling |
|
@afoures I would argue your suggestion solves a “problem” that is inconsequential. Importing In regard to conditional listeners, I think my proposal could support such a thing: function Foo(props: {
on?: Remix.EventListeners<HTMLButtonElement>
enableClicks?: boolean
}) {
return (
<button on={[
props.on,
props.enableClicks && on.click(event => {…}),
]}>
Click me
</button>
)
} |
|
@aleclarson how would you prevent this usage with TS on(signal, [
on.click((event) => {
event satisfies MouseEvent
event.type satisfies 'click'
event.currentTarget satisfies HTMLButtonElement
}),
]) |
|
@afoures I don't understand your question. In my proposal, type falsy = false | null | undefined
type DisposeFn = () => void
type EventDescriptor = {
type: string
listener: (event: Event) => void
options?: AddEventListenerOptions
}
declare function on(
target: EventTarget,
signalOrOptions: AbortSignal | AddEventListenerOptions | undefined,
descriptors: readonly (EventDescriptor | falsy)[]
): DisposeFn
on.click(listener) satisfies EventDescriptor
on.click(options, listener) satisfies EventDescriptor
on.click(target, listener) satisfies DisposeFn
on.click(target, options, listener) satisfies DisposeFnOf course, |
|
sorry, my question was: how can you make sure that typescript will only allow using the click factory for targets that emit at a certain point a but i have trouble seeing how your proposal would do that! |
|
another alternative with function could be type EventListenerFactory<target extends EventTarget> = {
[event in EventType<target>]: (
listener: ListenerFor<target, event>,
options?: { capture?: boolean; once?: boolean; passive?: boolean },
) => void;
};
function on<target extends EventTarget>(
target: target,
options: { signal: AbortSignal },
cb: (target: EventListenerFactory<target>) => void,
) {
// ...
}
const button = document.createElement("button");
const press = defineInteraction("custom:press", () => {});
declare global {
interface HTMLElementEventMap {
[press]: PointerEvent;
}
}
const controller = new AbortController();
on(button, { signal: controller.signal }, (target) => [
target.click((event) => {
// ...
}),
target.click(
(event) => {
// ...
},
{ capture: true },
),
target[press](
(event) => {
// ...
},
{ once: true },
),
target.focus(
(event) => {
// ...
},
{ passive: true },
),
]);
<div
on={(target) => [
target.click(
(event) => {
// ...
},
{ capture: true },
),
target.focus((event) => {
// not captured
}),
]}
/> |
This proposal has the following goals:
[interaction](event) {…}syntax for interaction listeningon.click(fn)) while keeping the new string-keyed API (e.g.{ click: fn })I believe these goals reflect the main concerns people have about the latest Events API (
@remix-run/[email protected]).1. Introduce
InteractiontypeChange
defineInteractionto return a type-safe function, instead of a string.This change…
interface HTMLElementEventMap {…}extensionsNote
Use of
satisfiesin this proposal is purely illustrative. You won't need it when using these APIs in your code. Read it as "this variable ABC is inferred to be of type XYZ".Usage
An example of using an interaction with a JSX element:
The
longPress()interaction returns a type-safe event descriptor:If the
...spread syntax feels jarring to you, note that you can nest it in an array instead. Before you roll your eyes, note that the newon()function (described in the next section) is yet another alternative syntax that you might prefer. The key here is to be flexible, as it lets developers choose the syntax that feels most natural to them, and it's more forgiving to agentic coding.2. Make
on()multi-purposeThe
on()function can be used 1 of 2 ways:When declaring event listeners with JSX, you don't provide an event target:
Importantly, you can still pass a listeners object to the
onprop. This API will feel more natural to beginners.Forwarding the
onpropYour components may want to accept an
onprop and forward it to a child JSX element. This is easy if we add nesting support. Essentially, the reconciler will flatten the array of listeners into a single object.Targeted
on()callsThe current
on()API is largely unchanged, but it now supports the same values as the new JSXonprop.When
on()receives an event target as the first argument, the listeners are immediately added to the target.