Ability to prevent/cancel fetch on focus

[jdanyow]

I'm using the remote input in a SelectMenu. The menu's SelectMenu-list is controlled by the remote input. When the page is rendered server-side, a few commonly used items are rendered in the SelectMenu-list to save the user time. The problem I have is these items are instantly cleared when the SelectMenu opens and the remote input receives focus due to the remote input doing the initial fetch.

Would the team be open to a PR that makes it possible to skip/cancel the initial fetch? I'd like to avoid the unnecessary round-trip to the server to get suggestions for the empty string query and preserve by server-side rendered default SelectMenu items. If yes, should it be done via html attribute or by preventing default on one of the custom events?

Two other questions:

1. Should load fire before the fetch starts? https://github.com/github/remote-input-element/blob/4ec740a69773f4b18dfeef28d52434b0475bcd13/src/index.ts#L132-L134
2. Most github elements store state in a weakmap and use module functions (as opposed to class properties and methods). This makes it difficult to monkey patch component behavior. Is that the intent?

Thank you for sharing the github elements. They're super useful and I love the decoupling of the styles from the behavior.

[keithamus]

Would the team be open to a PR that makes it possible to skip/cancel the initial fetch?

Yep sounds like a great addition :+1:

1. Should load fire before the fetch starts?

We try to follow the HTML Standard for this: https://html.spec.whatwg.org/#concept-media-load-algorithm. Media should be loaded roughly like this:

wait one "Task"
emit `loadstart`
fetch the data
wait one Task
if fetch caused error
  emit error
  emiit loadend
else
  emit load
  emit loadend

In this case it I believe load is executing at the right time. We also fire a loadstart event before the fetch - this is likely the event you want to target for the cancelable behaviour.

2. Most github elements store state in a weakmap and use module functions (as opposed to class properties and methods). This makes it difficult to monkey patch component behavior. Is that the intent?

This is certainly intentional. We try to expose public interfaces with intentionality. Having said that this project pre-dates our use of Private Fields which, if you are undertaking a refactor, I would encourage you to use over WeakMaps as they do offer better ergonomics.

If you wish to expose some state to allow for monkey patching, we're happy to discuss ideas - please provide a reasonable use case, so we can explore what an API might look like.

[jdanyow]

Thanks for the detailed response! Based on this it sounds like making loadstart cancellable and including the url in it's details would be a good start.

With regards to monkey patching, I don't have an urgent need other than the use-case here- preventing the empty string fetch. Cancelling loadstart is much nicer than monkey patching. Another scenario might be customizing the fetch/response handling, eg when a 401 occurs, the application might want to show some UI or silently refresh the auth state and retry.

I'm having some trouble with the package deps. Will troubleshoot further when I have some more time.

[keithamus]

Other elements we have assign a fetch function to themselves which allows for monkey patching (for example <include-fragment>). I'm not fully convinced about the pattern, however. I think having loadstart being cancelable would be better. I think we could also look to customize load to allow for customising the response data, but perhaps that's another discussion :wink:

The deps is likely because this element hasn't been updated in a while. I'm not sure where graphql as a dependency comes from, but please feel free to submit a PR updating the dependencies, if you are able to. If not let me know and I can take a look at it.