SelectInput relying on synthetic events, is there a workaround?

[c-dante]

Unsure if this is considered a bug or what, and I'm not convinced it should be up to y'all to address it, but the select component is relying on react's synthetic events to do the event.target replacement for option blur:

#10534
#12467

Specifically:
https://github.com/mui-org/material-ui/blame/master/packages/material-ui/src/Select/SelectInput.js#L156-L160

https://github.com/mui-org/material-ui/blame/master/packages/material-ui/src/Select/SelectInput.js#L128-L130

Event.target is a read only property on the DOM's event model. Some possible alternatives to this would be:

• Another param to eventing to carry that info
• Use the spread replacement that was there before (allowing dom events and synthetic events to work)
• Mutate the event itself with that extra info

I can also accept that none of that will happen and things like preact are just SOL with this particular widget.

[eps1lon]

Event.target is a read only property on the DOM's event model.

Yes but as you already said we're inside react which has its own synthetic event system.

Could you explain why we shouldn't rely on reacts event system or what the specific issue is with the current approach?

[c-dante]

Using a different virutal dom library that doesn't use synthetic events (IE preact, inferno, etc) and a compatibility layer won't work with the select widget (all other widgets work fine).

I 100% understand that it is not the goal of this library to consider those other use cases. I'm not suggesting that relying on the synthetic events is a problem at all -- that's probably the right way to work with react.

I'm suggesting that this current way of solving "have the events of the dropdown div be sourced from the select" by swapping out the target property might not be the best way to handle it. In particular, that shuts out non-synthetic events when using a compat layer (target is readOnly on native events), another solution could be generating the correct event triggered from the desired element (as opposed to hot-swapping the target).

If this is a non-starter, that's fine we can close it up. In our project we just switched over to using react. It just strikes me as strange that the select seems to be the only component to handle the source redirect in this manor.

[eps1lon]

Using a different virutal dom library that doesn't use synthetic events (IE preact, inferno, etc) and a compatibility layer won't work with the select widget (all other widgets work fine).

I didn't know that, thanks 👍 . We try to be compatible with preact though we're not running any tests with it. I think the biggest problem is that preact doesn't have its own event system? Which means there will be a lot of browser inconsistencies (most notably focus events bubbling). Overall I would agree that patching the target is not a good idea because you usually expect an html element there. I think in this particular case it makes integration with form libraries easier.

Could you add a repro that we can test against? A codesandbox would be ideal.

[oliviertassinari]

👍 for considering preact, it gets a bit of traction (as for inferno, I think that we should ignore it until it gets more users).

[c-dante]

Wow was not expecting the positive considerations! Thanks for the traction.

Here's a codesandbox demoing it failing:
https://codesandbox.io/s/preact-material-ui-select-broken-eles6?fontsize=14

I couldn't get JSX going with everything so I just wrote the small demo using preact's hyperscript method. I also added an input field to demo that other events work.

You can see in the browser's console the errors with setting the event target:

SelectInput.js:166 Uncaught TypeError: Cannot assign to read only property 'target' of object '#<MouseEvent>'
    at Object.eval [as click] (SelectInput.js:166)
    at HTMLLIElement.N (preact.js:1)
eval @ SelectInput.js:166
N @ preact.js:1

Here's the issue I made over @ prect: preactjs/preact#2031

[oliviertassinari]

Before we explore a potential solution, I would like to raise that using preact and the non-native select implementation seems misaligned. Material-UI provides a NativeSelect component that has a significantly smaller bundle size footprint and is free from this problem. I expect the audience that resonates with Preact to prefer the NativeSelect component.

That being said, if you still have a good reason for using the Select component, we would first need to avoid the read-only exception. Then, we need to think of a different API. I can think of two options. 1. In addition to target, we could use a new key, like fakeTarget. 2. We use the third argument of the onChange callback to give the user access to the needed information. 3. Suggestion welcome.

@eps1lon I'm curious to hear your point of view as you often raise great original solutions.

[eps1lon]

I think forms are very important and having a great integration story is more important than being able to support other targets.

In general I'd like to make it more visible that Select is not implementing the native intricacies (it couldn't since it supports objects as values).

I'd have to look at the current form library landscape and how integration works to make an educated guess. But then again if we do want to support preact seriously we might as well run the tests with it.

[c-dante]

I'm fairly certain it'd be possible to consume the event from the generated div dropdown and then emit the correct event from the select field. What are your thoughts on this?

Inside most of React's codebase, it seems they go down the route of creating new events when they need to perform this kind of indirection.

[glromeo]

While you decide on a strategic solution how about you just force writing on the target field of the event using something like:

Object.defineProperty(event, "target", {
  writable: true, 
  value: {
    ...
  }
});

I know it's (kind of) a hack but it would give us using preact a quick workaround while leaving the semantic as it is...

[eps1lon]

This specific issue was fixed in #18027.