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

Controlled input loses cursor under ShadowRoot in React 18 #597

Open
kimo-k opened this issue Nov 25, 2023 · 3 comments
Open

Controlled input loses cursor under ShadowRoot in React 18 #597

kimo-k opened this issue Nov 25, 2023 · 3 comments

Comments

@kimo-k
Copy link

kimo-k commented Nov 25, 2023

This controlled-input cursor bug appears when I try to use the new react.dom.client API to render an :input component within a shadow root. The text cursor skips to the end every time I insert a character.

The same component works fine with the react.dom API.

Is this expected, and is there anything I can do? I'm reticent to go with the react-native workaround, since my codebase is pretty large, with everything inside a shadow root (i.e. re-frame-10x).

Minimal repro: master...kimo-k:reagent:shadow-root-controlled-input-bug

kimo-k added a commit to day8/re-frame-10x that referenced this issue Nov 25, 2023
Reagent's built-in workaround for controlled inputs doesn't work when
we use the new `react.dom.client` API to render to a shadow root.

This makes the cursor skip to the end whenever you try to insert a
character into a text input field.

This article describes the bug in some detail:

https://github.com/reagent-project/reagent/blob/master/doc/ControlledInputs.md

It also offers a workaround. Basically, for our :input
components, instead of the :value prop (a controlled input), we use
:default-value (uncontrolled). Then, we have to force the :input
component to remount when we dispatch an event which would update the
:default-value. That allows us to "control" the value via a
subscription, while keeping the healthy cursor behavior of an
"uncontrolled" input.

So far, the only input we actually need to control is the new app-db
editor.

This is not a good smell, and could be a pitfall for future
development. I think it's tolerable for now, though. I hope reagent
can fix this issue - it seems really complicated.

reagent-project/reagent#597
@jaidetree
Copy link

Not sure if this fixes the issue but that code doesn't seem quite right.

It's creating a new input element every render cycle instead of updating an existing one. You could try adding a :key property to see if that improves the tracking. When I've ran into similar issues, it was usually because it was deleting the previous dom element and replacing it with a new one instead of updating it.

Lastly it seems like it's better to use with-let to initialize your atom in the repro component definition so you don't have to return a function. That way the atom is created only once in the component's lifecycle.

(defn repro []
  (with-let [text (r/atom "")]
    [:input {:on-change #(reset! text (.. % -target -value))
             :value @text}]))

Not sure that will entirely fix it, but it's a place to start.

@kimo-k
Copy link
Author

kimo-k commented Jan 24, 2024

Hey @jaidetree, thanks for taking a look. Were you able to run the repro without bugs?

I think your hypothesis might be incorrect, though. As far as I know, repro is an idiomatic form-2 component, :key would have no effect outside a seq, and using with-let will macroexpand to the same code.

@Deraen
Copy link
Member

Deraen commented Jan 24, 2024

@kimo-k Just note about :key. It isn't specific to sequences at all, any React component can have a key and React will use it to detect if the component identity changes. If identity changes, the previous component is unmounted and a new one created. This is different to just updating properties for same component instance. This can be useful outside of seqs when you need to reinitialize component local state (ratoms or hook state).

with-let implementation also does other stuff than just macroexpand into a basic form-2 component. Though it does indeed wrap the with-let body into (fn [] ...), i.e. expand into form-2 component.

Reagents controlled input cursor hacks are a huge mess and our async rendering batching for ratom changes is really problematic with React 18 update batching so there is probably no easy fix for this.

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