Conversation
Contributor
size-limit report 📦
|
Contributor
e2e tests |
Contributor
👀 Docs deployed
Commit a17ec0d |
7755cab to
8049364
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7285 +/- ##
==========================================
+ Coverage 83.94% 92.72% +8.77%
==========================================
Files 374 374
Lines 11108 11122 +14
Branches 3640 3651 +11
==========================================
+ Hits 9325 10313 +988
+ Misses 1783 809 -974
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
prevState может быть undefined, и типы теперь это показывают. Только не понятно как правильно этот undefined обрабатывать в компонентах где используется useCustomEnsuredControl.
when the callback is passed as nextValue
1070b31 to
213cbd3
Compare
inomdzhon
previously approved these changes
Aug 14, 2024
Contributor
inomdzhon
left a comment
There was a problem hiding this comment.
👏👏👏
Спасибо за исчерпывающее описание 🙏
BlackySoul
reviewed
Aug 16, 2024
Co-authored-by: Victoria Zhizhonkova <[email protected]>
inomdzhon
approved these changes
Aug 16, 2024
BlackySoul
approved these changes
Aug 21, 2024
vkcom-publisher
pushed a commit
that referenced
this pull request
Aug 22, 2024
… reveal prevValue undefined (#7285) Выяснилось, что всё-таки, если передавать в свойство `onChange` `useCustomEnsuredControl` коллбэк `nextValue` c аргументом `prevValue`, то есть вероятность, что `prevValue` будет `undefined`. Это видно, если исправить тип аргумента prevValue с `any` на `V`. https://github.com/VKCOM/VKUI/blob/a7b79b3b3a69bdf68163f62521f68697aceb52c2/packages/vkui/src/hooks/useEnsuredControl.ts#L56 Дело в том, что `preservedControlledValueRef`, которое мы передаём в `nextValue` коллбэк как значение `prevValue`, может быть `undefined`, так как хранит предыдущее значение `value`, которое тоже может быть `undefined`. https://github.com/VKCOM/VKUI/blob/a7b79b3b3a69bdf68163f62521f68697aceb52c2/packages/vkui/src/hooks/useEnsuredControl.ts#L70-L73 В теории (потому что в тестах не удалось повторить), если пользователь в `useCustomEnsuredControl` не которолирует значение `value` (то есть проп `value === undefined`, а мы храним значение `value` локально в `useCustomEnsuredControl`), а затем делает `value` котролируемым (то есть начинает передавать в проп `value` какое-то значение), то есть вероятность, что при вызове `onChanage` `preservedControlledValueRef` ещё не обновился и равен предыдущему значение `value`, то есть `undefined`, тогда пользователь получит `prevValue` со значением `undefined`, что может привести к ошибке, как описано в #7099 > TypeError: Uncaught TypeError: Cannot read properties of undefined (reading 'shown') ## Изменения - Исправили тип `prevValue` в колбэке `nextValue`, заменив `any`, чтобы была видна проблема c undefined. Но теперь ясно, что мы не можем просто так передать `preservedControlledValueRef` в `nextValue` коллбэк в качестве `prevValue`, потому что типы не сходятся. Что можно сделать. 1. Просто не вызывать `nextValue`, если `preservedControlledValueRef` `undefined`, но тогда мы получаем риск не отреагировать на действие пользователя, так как `onChange` не будет вызван и состояние не поменяется. 2. Разрешить `prevValue` по типам быть `undefined`. Но тогда пользователи, использующие в `onChange` коллбэк `nextValue` получат по типам вариант, что `prevValue` может быть `undefined`. Это очень странное состояние и тяжело объяснить его причины. А также тяжело принять решение при обработке onChange. Например, в `useChipsInput` не понятно откуда тогда взять актуальный массив `chips`, чтобы добавить в него новое значение. https://github.com/VKCOM/VKUI/blob/7755cabb003c2dac894703ac30335c0b874d8d52/packages/vkui/src/components/ChipsInput/useChipsInput.ts#L107-L126 3. (Текущее решение) Если `preservedControlledValueRef` `undefined`, то использовать значение `value`. Так как мы знаем, что компонент котролируемый, значит `value` имеет какое-то значение. При переходе из некоторолируемого значения в контролируемое у `value` нету предыдущего состояния, поэтому самым адекватным решением будет предать текущее значение `value` как `prevValue`. Чтобы `value` не находилось в массиве зависимостей `onChange` и не тригерило новое значение `onChange` при изменении, мы `value` прячем в `ref`, и используем лишь тогда, когда `preservedControlledValueRef` === `undefined`. - Добавили warning. Хотелось ещё дать ссылку на [React warning](https://react.dev/reference/react-dom/components/input#im-getting-an-error-a-component-is-changing-an-uncontrolled-input-to-be-controlled), тогда надо пояснять, что в нашем случае речь не только про input, а как это сделать лаконично я не придумал.
Contributor
| ✅ v6.5.2 🎉 |
andrey-medvedev-vk
pushed a commit
that referenced
this pull request
Sep 19, 2024
… reveal prevValue undefined (#7285) Выяснилось, что всё-таки, если передавать в свойство `onChange` `useCustomEnsuredControl` коллбэк `nextValue` c аргументом `prevValue`, то есть вероятность, что `prevValue` будет `undefined`. Это видно, если исправить тип аргумента prevValue с `any` на `V`. https://github.com/VKCOM/VKUI/blob/a7b79b3b3a69bdf68163f62521f68697aceb52c2/packages/vkui/src/hooks/useEnsuredControl.ts#L56 Дело в том, что `preservedControlledValueRef`, которое мы передаём в `nextValue` коллбэк как значение `prevValue`, может быть `undefined`, так как хранит предыдущее значение `value`, которое тоже может быть `undefined`. https://github.com/VKCOM/VKUI/blob/a7b79b3b3a69bdf68163f62521f68697aceb52c2/packages/vkui/src/hooks/useEnsuredControl.ts#L70-L73 В теории (потому что в тестах не удалось повторить), если пользователь в `useCustomEnsuredControl` не которолирует значение `value` (то есть проп `value === undefined`, а мы храним значение `value` локально в `useCustomEnsuredControl`), а затем делает `value` котролируемым (то есть начинает передавать в проп `value` какое-то значение), то есть вероятность, что при вызове `onChanage` `preservedControlledValueRef` ещё не обновился и равен предыдущему значение `value`, то есть `undefined`, тогда пользователь получит `prevValue` со значением `undefined`, что может привести к ошибке, как описано в #7099 > TypeError: Uncaught TypeError: Cannot read properties of undefined (reading 'shown') ## Изменения - Исправили тип `prevValue` в колбэке `nextValue`, заменив `any`, чтобы была видна проблема c undefined. Но теперь ясно, что мы не можем просто так передать `preservedControlledValueRef` в `nextValue` коллбэк в качестве `prevValue`, потому что типы не сходятся. Что можно сделать. 1. Просто не вызывать `nextValue`, если `preservedControlledValueRef` `undefined`, но тогда мы получаем риск не отреагировать на действие пользователя, так как `onChange` не будет вызван и состояние не поменяется. 2. Разрешить `prevValue` по типам быть `undefined`. Но тогда пользователи, использующие в `onChange` коллбэк `nextValue` получат по типам вариант, что `prevValue` может быть `undefined`. Это очень странное состояние и тяжело объяснить его причины. А также тяжело принять решение при обработке onChange. Например, в `useChipsInput` не понятно откуда тогда взять актуальный массив `chips`, чтобы добавить в него новое значение. https://github.com/VKCOM/VKUI/blob/7755cabb003c2dac894703ac30335c0b874d8d52/packages/vkui/src/components/ChipsInput/useChipsInput.ts#L107-L126 3. (Текущее решение) Если `preservedControlledValueRef` `undefined`, то использовать значение `value`. Так как мы знаем, что компонент котролируемый, значит `value` имеет какое-то значение. При переходе из некоторолируемого значения в контролируемое у `value` нету предыдущего состояния, поэтому самым адекватным решением будет предать текущее значение `value` как `prevValue`. Чтобы `value` не находилось в массиве зависимостей `onChange` и не тригерило новое значение `onChange` при изменении, мы `value` прячем в `ref`, и используем лишь тогда, когда `preservedControlledValueRef` === `undefined`. - Добавили warning. Хотелось ещё дать ссылку на [React warning](https://react.dev/reference/react-dom/components/input#im-getting-an-error-a-component-is-changing-an-uncontrolled-input-to-be-controlled), тогда надо пояснять, что в нашем случае речь не только про input, а как это сделать лаконично я не придумал.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
related: #7099
Описание
Выяснилось, что всё-таки, если передавать в свойство
onChangeuseCustomEnsuredControlколлбэкnextValuec аргументомprevValue, то есть вероятность, чтоprevValueбудетundefined.Это видно, если исправить тип аргумента prevValue с
anyнаV.VKUI/packages/vkui/src/hooks/useEnsuredControl.ts
Line 56 in a7b79b3
Дело в том, что
preservedControlledValueRef, которое мы передаём вnextValueколлбэк как значениеprevValue, может бытьundefined, так как хранит предыдущее значениеvalue, которое тоже может бытьundefined.VKUI/packages/vkui/src/hooks/useEnsuredControl.ts
Lines 70 to 73 in a7b79b3
В теории (потому что в тестах не удалось повторить), если пользователь в
useCustomEnsuredControlне которолирует значениеvalue(то есть пропvalue === undefined, а мы храним значениеvalueлокально вuseCustomEnsuredControl), а затем делаетvalueкотролируемым (то есть начинает передавать в пропvalueкакое-то значение), то есть вероятность, что при вызовеonChanagepreservedControlledValueRefещё не обновился и равен предыдущему значениеvalue, то естьundefined, тогда пользователь получитprevValueсо значениемundefined, что может привести к ошибке, как описано в #7099Изменения
prevValueв колбэкеnextValue, заменивany, чтобы была видна проблема c undefined.Но теперь ясно, что мы не можем просто так передать
preservedControlledValueRefвnextValueколлбэк в качествеprevValue, потому что типы не сходятся.Что можно сделать.
Просто не вызывать
nextValue, еслиpreservedControlledValueRefundefined, но тогда мы получаем риск не отреагировать на действие пользователя, так какonChangeне будет вызван и состояние не поменяется.Разрешить
prevValueпо типам бытьundefined. Но тогда пользователи, использующие вonChangeколлбэкnextValueполучат по типам вариант, чтоprevValueможет бытьundefined.Это очень странное состояние и тяжело объяснить его причины. А также тяжело принять решение при обработке onChange.
Например, в
useChipsInputне понятно откуда тогда взять актуальный массивchips, чтобы добавить в него новое значение.VKUI/packages/vkui/src/components/ChipsInput/useChipsInput.ts
Lines 107 to 126 in 7755cab
(Текущее решение) Если
preservedControlledValueRefundefined, то использовать значениеvalue.Так как мы знаем, что компонент котролируемый, значит
valueимеет какое-то значение. При переходе из некоторолируемого значения в контролируемое уvalueнету предыдущего состояния, поэтому самым адекватным решением будет предать текущее значениеvalueкакprevValue.Чтобы
valueне находилось в массиве зависимостейonChangeи не тригерило новое значениеonChangeпри изменении, мыvalueпрячем вref, и используем лишь тогда, когдаpreservedControlledValueRef===undefined.Можно оставить как есть и добавить warning, как в реакте A component is changing an uncontrolled input to be controlled.
В целом, это рабочий вариант, и ворнинг можно добавить к любому из остальных решений.
Другое дело, что он будет менее понятен, так как хук будет использоваться внутри компонентов и сообщение будет абстрактным,и вроде как лучше бы не нагружать пользователя лишней информацией.
Но всё же в этом есть смысл.