From 7f2865f06a990ea23fa2a82b8231b19e8f85f504 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 18 May 2023 10:22:55 +0200 Subject: [PATCH 1/2] Revert "add delay by 1 microtick (#4005)" This reverts commit bcf365be26dc658eb65e06072ffa8cb3295f60c1. --- compat/src/render.js | 4 ++-- compat/test/browser/controlledInput.test.js | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/compat/src/render.js b/compat/src/render.js index 457c7b8c91..74501172f5 100644 --- a/compat/src/render.js +++ b/compat/src/render.js @@ -118,10 +118,10 @@ options.event = e => { */ function afterEvent(eventType, target) { if (target.value != null) { - Promise.resolve().then(() => (target.value = target._prevValue)); + target.value = target._prevValue; } if (eventType === 'change' && target.checked != null) { - Promise.resolve().then(() => (target.checked = target._prevValue)); + target.checked = target._prevValue; } } diff --git a/compat/test/browser/controlledInput.test.js b/compat/test/browser/controlledInput.test.js index b529db267f..9f14009412 100644 --- a/compat/test/browser/controlledInput.test.js +++ b/compat/test/browser/controlledInput.test.js @@ -168,7 +168,7 @@ describe('preact/compat controlled inputs', () => { render(, scratch); scratch.firstChild.value = 'A'; - await fireEvent(scratch.firstChild, 'change'); + fireEvent(scratch.firstChild, 'change'); expect(calls).to.deep.equal(['A']); expect(scratch.firstChild.value).to.equal('A'); @@ -208,8 +208,6 @@ describe('preact/compat controlled inputs', () => { scratch.firstChild.checked = false; await fireEvent(scratch.firstChild, 'change'); - // Have to wait for the microtick - await new Promise(res => setTimeout(res)); expect(calls).to.deep.equal([false]); expect(scratch.firstChild.checked).to.equal(true); }); From 0bd78368ff5e11d89c985186c6e8b948df5883b1 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 18 May 2023 10:26:57 +0200 Subject: [PATCH 2/2] revert controlled components --- compat/src/render.js | 57 ------ compat/test/browser/controlledInput.test.js | 214 -------------------- demo/controlled-inputs.jsx | 19 -- demo/index.jsx | 5 - 4 files changed, 295 deletions(-) delete mode 100644 compat/test/browser/controlledInput.test.js delete mode 100644 demo/controlled-inputs.jsx diff --git a/compat/src/render.js b/compat/src/render.js index 74501172f5..b215c60065 100644 --- a/compat/src/render.js +++ b/compat/src/render.js @@ -87,44 +87,12 @@ let oldEventHook = options.event; options.event = e => { if (oldEventHook) e = oldEventHook(e); - /** @type {ControlledTarget} */ - const target = e.currentTarget; - const eventType = e.type; - if ( - (eventType === 'input' || eventType === 'change') && - target._isControlled - ) { - // Note: We can't just send the event to the afterEvent function because - // some properties on the event (e.g. currentTarget) will be changed by the - // time afterEvent is called. `currentTarget` will be `null` at that point. - // The browser reuses event objects for event handlers and just modifies the - // relevant properties before invoking the next handler. So whenever we call - // afterEvent, if we were to inspect the original Event object, we would see - // that the currentTarget is null. So instead we pass the event type and the - // target to afterEvent. - Promise.resolve().then(() => afterEvent(eventType, target)); - } - e.persist = empty; e.isPropagationStopped = isPropagationStopped; e.isDefaultPrevented = isDefaultPrevented; return (e.nativeEvent = e); }; -/** - * @typedef {EventTarget & {value: any; checked: any; _isControlled: boolean; _prevValue: any}} ControlledTarget - * @param {string} eventType - * @param {ControlledTarget} target - */ -function afterEvent(eventType, target) { - if (target.value != null) { - target.value = target._prevValue; - } - if (eventType === 'change' && target.checked != null) { - target.checked = target._prevValue; - } -} - function empty() {} function isPropagationStopped() { @@ -278,33 +246,8 @@ options.diffed = function (vnode) { oldDiffed(vnode); } - const type = vnode.type; const props = vnode.props; const dom = vnode._dom; - const isControlled = dom && dom._isControlled; - - if ( - dom != null && - (type === 'input' || type === 'textarea' || type === 'select') - ) { - if (isControlled === false) { - } else if ( - isControlled || - props.oninput || - props.onchange || - props.onChange - ) { - if (props.value != null) { - dom._isControlled = true; - dom._prevValue = props.value; - } else if (props.checked != null) { - dom._isControlled = true; - dom._prevValue = props.checked; - } else { - dom._isControlled = false; - } - } - } if ( dom != null && diff --git a/compat/test/browser/controlledInput.test.js b/compat/test/browser/controlledInput.test.js deleted file mode 100644 index 9f14009412..0000000000 --- a/compat/test/browser/controlledInput.test.js +++ /dev/null @@ -1,214 +0,0 @@ -import { - setupScratch, - teardown, - createEvent -} from '../../../test/_util/helpers'; - -import React, { - render, - createElement, - Component, - useState -} from 'preact/compat'; - -describe('preact/compat controlled inputs', () => { - /** @type {HTMLDivElement} */ - let scratch; - - /** - * @param {EventTarget} on - * @param {string} type - * @returns {Promise} - */ - function fireEvent(on, type) { - let e = createEvent(type); - on.dispatchEvent(e); - // Flush the microtask queue after dispatching an event by returning a - // Promise to mimic what the browser would do after invoking event handlers. - // Technically, this test does it only after all event handlers have been - // invoked, whereas a real event dispatched by a browser would do it after - // each event handler. - return Promise.resolve(); - } - - beforeEach(() => { - scratch = setupScratch(); - }); - - afterEach(() => { - teardown(scratch); - }); - - it('should support controlled inputs', async () => { - const calls = []; - class Input extends Component { - constructor(props) { - super(props); - this.state = { text: '' }; - this.onInput = this.onInput.bind(this); - } - - onInput(e) { - calls.push(e.target.value); - if (e.target.value.length > 3) return; - this.setState({ text: e.target.value }); - } - - render() { - return ; - } - } - - render(, scratch); - - scratch.firstChild.value = 'hii'; - await fireEvent(scratch.firstChild, 'input'); - expect(calls).to.deep.equal(['hii']); - expect(scratch.firstChild.value).to.equal('hii'); - - scratch.firstChild.value = 'hiii'; - await fireEvent(scratch.firstChild, 'input'); - expect(calls).to.deep.equal(['hii', 'hiii']); - expect(scratch.firstChild.value).to.equal('hii'); - }); - - it('should support controlled inputs with bailed out rerenders', async () => { - const calls = []; - function Input() { - const [value, setValue] = useState(''); - return ( - { - calls.push(e.target.value); - setValue(e.target.value.toUpperCase().slice(0, 3)); - }} - /> - ); - } - - render(, scratch); - - scratch.firstChild.value = 'hii'; - await fireEvent(scratch.firstChild, 'input'); - expect(calls).to.deep.equal(['hii']); - expect(scratch.firstChild.value).to.equal('HII'); - - scratch.firstChild.value = 'hiii'; - await fireEvent(scratch.firstChild, 'input'); - expect(calls).to.deep.equal(['hii', 'hiii']); - expect(scratch.firstChild.value).to.equal('HII'); - - scratch.firstChild.value = 'ahiii'; - await fireEvent(scratch.firstChild, 'input'); - expect(calls).to.deep.equal(['hii', 'hiii', 'ahiii']); - expect(scratch.firstChild.value).to.equal('AHI'); - }); - - it('should support controlled textareas', async () => { - const calls = []; - class Input extends Component { - constructor(props) { - super(props); - this.state = { text: '' }; - this.onInput = this.onInput.bind(this); - } - - onInput(e) { - calls.push(e.target.value); - if (e.target.value.length > 3) return; - this.setState({ text: e.target.value }); - } - - render() { - return