Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: 注册异步渲染器时删除占位渲染器,修复设置了显隐条件时无法正确渲染的问题 #11544

Conversation

BeMxself
Copy link
Contributor

优化异步渲染器注册逻辑

What

  • 当已存在占位渲染器时,在注册异步渲染器时删除占位渲染器

Why

  • 解决了在某些情况下(如设置了 visibleOn/hiddenOn 条件)无法渲染的问题

Copy link

👍 Thanks for this!
🏷 I have applied any labels matching special text in your issue.

Please review the labels and make any necessary changes.

@github-actions github-actions bot added the fix label Jan 24, 2025
Song Mingxu added 2 commits January 26, 2025 19:45
- 当已存在占位渲染器时,在注册异步渲染器时删除占位渲染器
- 解决了在某些情况下(如设置了 visibleOn/hiddenOn 条件)无法渲染的问题
@BeMxself BeMxself force-pushed the fix-delete-placeholder-when-register-async-renderer branch from 88b4eec to f06c88f Compare January 26, 2025 13:00
@BeMxself
Copy link
Contributor Author

BeMxself commented Feb 3, 2025

过年好啊!
年前我在检查代码的时候发现大量的测试用例过不去,因为之前的逻辑会导致一些隐含了spinner的组件在跑测试用例的时候不出现,增加这块逻辑后spinner就会被渲染了,需要在很多测试用例中添加下面这样的代码:

  await waitForElementToBeRemoved(() =>
    container.querySelector('.cxd-Spinner')
  );

我改了一部分,大部分的测试用例都可以通过这种方式修复,但是还是有少部分的dom跟快照对不起来。

@BeMxself
Copy link
Contributor Author

BeMxself commented Feb 5, 2025

过年好啊! 年前我在检查代码的时候发现大量的测试用例过不去,因为之前的逻辑会导致一些隐含了spinner的组件在跑测试用例的时候不出现,增加这块逻辑后spinner就会被渲染了,需要在很多测试用例中添加下面这样的代码:

  await waitForElementToBeRemoved(() =>
    container.querySelector('.cxd-Spinner')
  );

我改了一部分,大部分的测试用例都可以通过这种方式修复,但是还是有少部分的dom跟快照对不起来。

想听听维护者的意见,是大面积修改测试用例还是用什么trick欺骗jest?(感觉后者不太好)

@2betop
Copy link
Member

2betop commented Feb 5, 2025

抱歉,前面我弄错了,原始对象这里其实直接删除会更方便点。renderer 上删除了 placeholder component ,需要 renderers 数组里面把那个旧的替换成新的,索性还不如直接操作原始对象执行删除!

@2betop 2betop merged commit b49f7b6 into baidu:master Feb 5, 2025
3 checks passed
@2betop
Copy link
Member

2betop commented Feb 5, 2025

感谢,已经合入

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants