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

feat(useWatch): notify watch in preserve mode #577

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

RexSkz
Copy link
Contributor

@RexSkz RexSkz commented Mar 7, 2023

The existing prop preserve in Form is used to keep values even if the field is not rendered. However, it will not trigger the watchList because the getFieldsValue is called without true and will return only the visible fields.

Form 目前已有的 preserve 属性是用来在 field 没有被渲染的情况下依旧能获取值。但它不能触发 watchList,因为内部的 getFieldsValue 在不传 true 的情况下只会返回可见的 fields。

@vercel
Copy link

vercel bot commented Mar 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
field-form ❌ Failed (Inspect) Mar 10, 2023 at 6:45AM (UTC)

@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #577 (c2cd323) into master (7bd75b5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head c2cd323 differs from pull request most recent head b2a2154. Consider uploading reports for the commit b2a2154 to get more accurate results

@@           Coverage Diff           @@
##           master     #577   +/-   ##
=======================================
  Coverage   99.82%   99.82%           
=======================================
  Files          16       16           
  Lines        1143     1148    +5     
  Branches      256      260    +4     
=======================================
+ Hits         1141     1146    +5     
  Misses          2        2           
Impacted Files Coverage Δ
src/Field.tsx 100.00% <ø> (ø)
src/useForm.ts 99.76% <100.00%> (+<0.01%) ⬆️
src/useWatch.ts 97.77% <100.00%> (+0.15%) ⬆️
src/utils/typeUtil.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zombieJ
Copy link
Member

zombieJ commented Mar 8, 2023

这会有点怪异,什么情况下被依赖的项不见了,同时依赖项还要更新的?能举个例子么?

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 8, 2023

例如表单有两个 step,不同 step 渲染的 fields 不同,但 step 1 的某字段值改变后会影响到 step 2 的一些字段的值。

此外还有一个场景,就是根据数组来渲染 fields,例如这个场景:

React.useEffect(() => {
  form.setFieldsValue({
    items: [
      { name: 'a' },
      { name: 'b' },
    ],
  });
}, []);

const items = Form.useWatch('items', form) || [];
return items.map((item, index) => (
  <Field name={['items', index]} key={`${items}-${index}`}>
    ...
  </Field>
));

在一开始并没有 Field 被渲染,因此即使点击了某个 + Add 按钮时调用了 setFieldsValue 加了一个 item 进去,页面也不会更新。

src/useForm.ts Outdated Show resolved Hide resolved
@zombieJ
Copy link
Member

zombieJ commented Mar 8, 2023

交互不太直观,但是听起来需求是合理的。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 8, 2023

我感觉应该是 ok 的,这个改动不影响现有设计和实现,不会给未来留坑,让 useWatch 符合了 preserve 的预期,甚至还有性能提升(传入 true 会直接返回 store 引用而不是经过处理)😂

可以再举一个需求场景:有一个巨型表单页面,我们会因为性能问题而设计成“点击某个面板展开对应的表单区域”,那么未展开的区域内字段 watch 就不生效了。

@zombieJ
Copy link
Member

zombieJ commented Mar 8, 2023

逻辑我理一下,以防万一

@zombieJ
Copy link
Member

zombieJ commented Mar 8, 2023

想了想放 Form 的 preserve 做触发不太适合,useWatch 看起来可以多接受一个 config param 来对任意变化都监听。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 8, 2023

Form.useWatch('name', form, { 不知道起啥名: true })

不过这参数不太好传进 notifyWatch,因为参数是跟着每个 watch 走的,而 getFieldsValue 是在 watchList 之外执行的,也就不会受到 watch 参数的控制。不知道这个问题如何解决会好一些?

BTW,我发现当前的修改对于 initialValues 是不生效的(watch 一个隐藏 field 第一次一定返回 undefined),原因是 useWatch 这里 少加了 true;这个地方获取不到 Form 的 preserve,但如果把参数放到 useWatch 里面的话则可以解决。

@zombieJ
Copy link
Member

zombieJ commented Mar 9, 2023

可以是可变参数:

Form.useWatch('name')

Form.useWatch('name', form)

Form.useWatch('name', { form, preserve: true })

@zombieJ
Copy link
Member

zombieJ commented Mar 9, 2023

另外 bug 的话一步一步来,分几个 PR 搞会清楚一些~

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

可以是可变参数:

Form.useWatch('name')

Form.useWatch('name', form)

Form.useWatch('name', { form, preserve: true })

感觉把 formpreserve 合成一个参数有点怪,一个是增加了函数内部的实现成本,一个是 nameform 为必传项(或者说是主体),而 preserve 是可选项(或者说是修饰符)。

与之类似的函数 addEventListener 就是三个参数:ellisteneroptions,所以我觉得三个参数会好一些。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

Anyway,我先试着写一下,思路大概是这样,你看有没有什么问题:

  • 要改两个地方:useWatch(store 改变时需要触发 watch)和 useForminitialValues 也需要触发 watch)。
  • useForm 很好改,直接根据新的 preserve 参数来判断要不要加 true
  • useWatch 则需要把新的 preserve 参数想办法传到 watchList 中,然后同时用两份数据源,每个 watch 根据其自身的 preserve 判断使用哪一份数据源。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

@zombieJ 实现应该是没有问题的,不过调用方式我还是觉得加一个新的参数会比较好,就按照这样来写了😂

@li-jia-nan
Copy link
Member

@zombieJ 这个仓库的 CI 好像偶尔不会跑,是要手动触发一下吗?

@zombieJ
Copy link
Member

zombieJ commented Mar 9, 2023

@zombieJ 实现应该是没有问题的,不过调用方式我还是觉得加一个新的参数会比较好,就按照这样来写了😂

新加参数容易变成参数地狱,如果未来 useWatch 又需要新的参数,而且又比 preserve 更重要,那也不得不放在 preserve 后面。对于 form 而言,本身是可选的,preserve 也是可选的,那就没有必要再一路加上去了。

@zombieJ
Copy link
Member

zombieJ commented Mar 9, 2023

@zombieJ 这个仓库的 CI 好像偶尔不会跑,是要手动触发一下吗?

first time contributor 需要 owner 手工点确认执行以防止有恶意代码注入,这个是 github 的安全机制。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

@zombieJ 实现应该是没有问题的,不过调用方式我还是觉得加一个新的参数会比较好,就按照这样来写了😂

新加参数容易变成参数地狱,如果未来 useWatch 又需要新的参数,而且又比 preserve 更重要,那也不得不放在 preserve 后面。对于 form 而言,本身是可选的,preserve 也是可选的,那就没有必要再一路加上去了。

虽然在函数签名中 form 是可选的,但其实如果不传的话会报 warning 并且在之后不做任何实际的事情,从这一点上来看它其实是必传项,且其重要程度跟 namePath 是等价的。

之所以设计成三个参数,其实就是仿照 addEventListener,降低学习成本。如果有新的参数,是可以放到跟 preserve 一级,例如未来某天可以这样用:

Form.useWatch('name', form, {
  preserve: true,
  passive: true,
});

@zombieJ
Copy link
Member

zombieJ commented Mar 9, 2023

虽然在函数签名中 form 是可选的,但其实如果不传的话会报 warning 并且在之后不做任何实际的事情,从这一点上来看它其实是必传项,且其重要程度跟 namePath 是等价的。

只有和 Form 同级的时候才需要传入。如果是 Form 内部的组件,是会通过 context 获取 FormInstance 的:
https://github.com/react-component/field-form/blob/master/src/useWatch.ts#L82

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

原来在上面还有个逻辑啊,我了解了,这就改一下。

@RexSkz
Copy link
Contributor Author

RexSkz commented Mar 9, 2023

可以了,为了判断参数类型(都是 Object),额外写了个 isFormInstance 的方法。

src/interface.ts Outdated

export interface WatchItem {
callback: WatchCallBack;
options: WatchOptions;
Copy link
Contributor Author

@RexSkz RexSkz Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里以及下面的地方都是内部使用,如果可能的话我一般不会省略,这可以避免因为“改变数据结构/函数签名但漏改实现”而导致的问题。
数据结构还好,主要是函数签名,如果我有个地方忘记传 options,ts 会帮我检验出这个问题,如果设置成可选的,ts 就帮不上忙了。

src/useForm.ts Outdated

private registerWatch: InternalHooks['registerWatch'] = callback => {
this.watchList.push(callback);
private registerWatch: InternalHooks['registerWatch'] = (callback, options) => {
Copy link
Member

@zombieJ zombieJ Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不用在 registerWatch 里走一圈 options,本身就是 useWatch 的参数。在遍历 watchList 的时候 callback 带上保留值 和 全部 值即可,让 useWatch 自行处理:

this.watchList.forEach(callback => {
  callback(values, allValues, namePath);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

有道理,这样 options 就被限制在 useWatch 里面了,我改一下~

@zombieJ zombieJ merged commit 680c534 into react-component:master Mar 13, 2023
@zombieJ
Copy link
Member

zombieJ commented Mar 13, 2023

+ [email protected]

antd 侧也需要给 feature 发个 PR 提升一下哈~

@closertb
Copy link

closertb commented Jun 27, 2023

image
我这个项目几个月没改过项目代码,就因为额外升级了包,导致rc-field-form升级,结果触发一个bug,找了半天,发现是这个更新造成的,这个改动貌似是有点草率:
image

@RexSkz
Copy link
Contributor Author

RexSkz commented Jun 27, 2023

image 我这个项目几个月没改过项目代码,就因为额外升级了包,导致rc-field-form升级,结果触发一个bug,找了半天,发现是这个更新造成的,这个改动貌似是有点草率: image

这个修改应该是向后兼容的,只有在 useWatch 的参数中配置了 preserve: true 才会有新的行为。如果误改到了旧行为,测试会报错。

可以提供一个复现用例吗?我看看是不是漏掉了什么场景。

@closertb
Copy link

closertb commented Jun 27, 2023

image 我这个项目几个月没改过项目代码,就因为额外升级了包,导致rc-field-form升级,结果触发一个bug,找了半天,发现是这个更新造成的,这个改动貌似是有点草率: image

这个修改应该是向后兼容的,只有在 useWatch 的参数中配置了 preserve: true 才会有新的行为。如果误改到了旧行为,测试会报错。

可以提供一个复现用例吗?我看看是不是漏掉了什么场景。

最后发现不是preserve 造成的,是 isListField 这个判断造成:

  // 这里初始化时, 正常时拿到的是 null, 但那个错误包版本拿到的是 { f(getKeys) }
  const wrapperListContext = React.useContext(ListContext);

  const listContext = React.useMemo<ListContextProps>(
    () => ({
      getKey: (namePath: InternalNamePath) => {
        const len = prefixName.length;
        const pathName = namePath[len];
        return [keyManager.keys[pathName], namePath.slice(len + 1)];
      },
    }),
    [prefixName],
  );


    <ListContext.Provider value={listContext}>
      <FieldContext.Provider value={fieldContext}>
        <Field
          name={[]}
          shouldUpdate={shouldUpdate}
          rules={rules}
          validateTrigger={validateTrigger}
          initialValue={initialValue}
          isList
          isListField={isListField ?? !!wrapperListContext}
        >
<.....>

由于项目依赖太多,rc-field-form 版本太多,直接回退了 package-lock。没有找具体是那个版本引起的

后面又查了下,1.27.4是正常的,更新到1.30 就稳定复现

@dawufenfen

This comment was marked as resolved.

@RexSkz
Copy link
Contributor Author

RexSkz commented Feb 23, 2024

useWatch的参数中,第二个参数(即form)可以传undefined的,但是本次改动没考虑到,因而出现了form为空导致useWatch取form报错引发白屏。 😵

function useWatch(
  ...args: [NamePath | ((values: Store) => any), FormInstance | WatchOptions<FormInstance>]
) {
 const [dependencies, _form = {}] = args;
 const options = isFormInstance(_form) ? { form: _form } : _form;  
 const form = options.form;  //options在form传undefined时为undefined。
  ...
}

不想升级的话可以用useWatch(xxx, { form })代替原本的useWatch(xxx, form)

options 不会是 undefined。显式传入 undefined 时会走到 _form = {} 的默认值逻辑,最后可以正确获取到 form === undefined

image

@dawufenfen
Copy link

dawufenfen commented Feb 27, 2024

useWatch的参数中,第二个参数(即form)可以传undefined的,但是本次改动没考虑到,因而出现了form为空导致useWatch取form报错引发白屏。 😵

function useWatch(
  ...args: [NamePath | ((values: Store) => any), FormInstance | WatchOptions<FormInstance>]
) {
 const [dependencies, _form = {}] = args;
 const options = isFormInstance(_form) ? { form: _form } : _form;  
 const form = options.form;  //options在form传undefined时为undefined。
  ...
}

不想升级的话可以用useWatch(xxx, { form })代替原本的useWatch(xxx, form)

options 不会是 undefined。显式传入 undefined 时会走到 _form = {} 的默认值逻辑,最后可以正确获取到 form === undefined

image

是的,抱歉,当时排查看到useWatch实现里的args定义里没有undefined,就以为是undefined带来的问题(不过定义是不是加上比较好?),没有再细看了。
实际上我的业务场景里的bug是因为产生了传入form为null的情况,同时添加了类型断言为FormInstance。 >_< 而5.4.0之前的版本在form强行传入null时并不会报错,因此在升级到5.4.0之后在options.form这里才触发了报错。

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

Successfully merging this pull request may close these issues.

5 participants