change argument order for on function with AbortSignal

[kentcdodds]

It's easier to make a PR about this than an issue/discussion. Just curious what you think about doing this to avoid unfortunate function overloading. Overloading is irritating as a consumer of the types. Having an optional options argument at the end seems reasonable.

[mjackson]

I actually had a similar thought about the 2nd signal arg. In addition to eliminating the function overload, this API would provide a convenient spot to put other addEventListener options, so you could do stuff like this:

on(button, { keydown }, { capture: true, once: true })

// or even
on(button, { keydown }, true) // useCapture

The 3rd arg would essentially be the exact same as the 3rd arg to addEventListener, either options or a useCapture boolean, which makes on feel like a little more like an enhancement on top of addEventListener than a replacement.

[ryanflorence]

The API is mostly in service to creating elements with JSX. How would you do this?

<div
  on={{
    click: capture(() => {
      // ...
    }),
    focus: () => {
      // not captured
    },
  }}
/>

[ryanflorence]

The reason the signal is the optional second arg is just because it's butt ugly with prettier when it's the last arg

```tsx
function ComponentWithWindowEvents(this: Remix.Handle) {
  on(window, this.signal, {
    scroll: () => {},
    focus: capture(() => {}),
    beforeunload: () => {},
  })

  return () => {
    // ...
  }
}

function ComponentWithWindowEvents(this: Remix.Handle) {
  on(
    window,
    {
      scroll: () => {},
      focus: capture(() => {}),
      beforeunload: () => {},
    },
    { signal: this.signal },
  )

  return () => {
    // ...
  }
}

While I originally was thinking of this as a thin wrapper over addEventListener it's more about a declarative way to add multiple events to a single target with different options all at once and to be able to clean them up all up all at once as well.

[brookslybrand]

For what it's worth, I had the same thought when reading the docs yesterday, but thought I might just be missing something.

If the main reason is because of the prettier formatting ugliness, I personally don't think that's enough of a reason to not make the signal the 3rd argument.

I also don't mind how it reads that badly, and if I really did care that much I'd just curry it and call onWindow 😉

onWindow(
  {
    scroll: () => {},
    focus: capture(() => {}),
    beforeunload: () => {},
  },
  { signal: this.signal },
)

Either way, a 2nd argument being the optional argument feels really weird on top of it deviating a bit in structure from what devs will be used to with addEventListener. Even if it's not necessarily a wrapper of addEventListener, I think maintaining similar argument order is nice.

[aleclarson]

The concern of this PR would be obviated if there was a this.on() method on Remix.Handle. I haven't seen this idea discussed anywhere.

Essentially, this.signal would be added automatically.

this.on(target, { ...listeners })

As far as allowing AddEventListenerOptions as a third argument, I say “why not?” – No reason not to support both ways of injecting listener options, since the capture()/listenWith() functions are still useful with JSX, as Ryan pointed out.

[mjackson]

The concern of this PR would be obviated if there was a this.on() method on Remix.Handle.

I agree with @aleclarson. Ryan and I have discussed a potential this.on API, but haven't finalized anything yet.

I do like the conceptual symmetry between this.on and <button on>. Both this (Remix element) and <button> (DOM element) have a "disconnected" event that we can plug into to automatically remove listeners.