ByzantineFailure / BPM-for-Discord

BPM for Discord's Desktop App. Includes one-click installers, update notifications, and custom script support.
GNU Affero General Public License v3.0
17 stars 8 forks source link

Emotes inserted from the search box are cleared when the textbox is selected. #92

Closed DXHHH101 closed 6 years ago

DXHHH101 commented 6 years ago

If you add an emote from the search box, it's fine, the emote is in the textbox. But as soon as you click the textbox to send the message, all emote text clears from the textbox instantly (only the emotes, if you've written something else in the box as well, it stays). This is on the latest version of BPM for Better Discord.

PurpleMyst commented 6 years ago

Can confirm the issue happens with the regular Discord client as well.

ByzantineFailure commented 6 years ago

Repro'd on my end. I suspect there's some onkeyup logic or something similar that tracks textbox state that's never fired when the text is inserted programmatically. I don't know how to solve this, but I'll see what I can do.

ByzantineFailure commented 6 years ago

After several hours digging through pretty-printed, minified Discord and React source while setting all sorts of breakpoints, what's going on is:

Since React 16 (which Discord updated to at an eerily similar time as my last release), when you update the value of a particular DOM node programmatically it looks like there's some logic that updates React's internal state handler for the node's value WITHOUT dispatching an event to the top-level event handler which tells React components to update their state/props.

Then, when you do any input (or focus/blur), it re-renders the component against its internal state and bam, your emote text is gone.

If I set the value and trigger an event, the internal value tracker doesn't recognize anything is incorrect due to it already having the correct value. But if I trigger the event without setting the value the event won't have any changes to pick up.

I have no idea how to fix this one but I've got a root cause now.

ByzantineFailure commented 6 years ago

https://github.com/facebook/react/blob/e932ad68bed656eed5295b61ba74e5d0857902ed/src/renderers/dom/shared/inputValueTracking.js#L75

Mad.

ByzantineFailure commented 6 years ago
function doTheThing(node, val) {
    var oldValue = node.value;
    delete node.value;
    node.value = val;
    var event = new Event('change', { target: node, bubbles: true, cancelable: false });
    node.dispatchEvent(event);
}

HAAAAAHAHAHA, FUCK YOU, REACT.

We can get away with this because: https://github.com/facebook/react/blob/e932ad68bed656eed5295b61ba74e5d0857902ed/src/renderers/dom/shared/inputValueTracking.js#L63

ByzantineFailure commented 6 years ago

Fixed, will push out an update soon.

ByzantineFailure commented 6 years ago

Realistically speaking we should do something with Object.getOwnPropertyDescriptor to preserve and reset the actual listeners from React after we're done updating things, but tbh I'm pretty peeved about this whole business and given that nothing is broken as far as I can tell, am just going to let it lie.