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

fix: form validate resolve value not satisfy scope isolation #1710

Merged
merged 1 commit into from Jul 18, 2023

Conversation

pointhalo
Copy link
Collaborator

@pointhalo pointhalo commented Jul 18, 2023

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Test Case
  • TypeScript definition update
  • Document improve
  • CI/CD improve
  • Branch sync
  • Other, please describe:

PR description

formApi.validate.then( (values) => {
   
})

入参中的 values 未做作用域隔离,用户若直接读取了 values,Field 对应的 DOM 若卸载了,这里的values的值也会被卸载。应该为一个不变的快照才对。

所以增加输出前的 deepClone环节

Changelog

🇨🇳 Chinese

  • Fix: 修复 Form validate.then() 中的 values 入参未做作用域隔离,会受到 Field DOM 挂载、卸载影响的问题

🇺🇸 English

  • Fix: Fix the problem that the values input parameter in Form validate.then() is not scope isolated and will be affected by Field DOM mount and unmount

Checklist

  • Test or no need
  • Document or no need
  • Changelog or no need

Other

  • Skip Changelog

Additional information

@codesandbox-ci
Copy link

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.

Latest deployment of this branch, based on commit 6179b63:

Sandbox Source
pr-story Configuration
Semi Design: Simple Story Configuration

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.06 ⚠️

Comparison is base (6ed4f82) 88.13% compared to head (5be7b1f) 88.07%.

❗ Current head 5be7b1f differs from pull request most recent head 6179b63. Consider uploading reports for the commit 6179b63 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1710      +/-   ##
==========================================
- Coverage   88.13%   88.07%   -0.06%     
==========================================
  Files         433      433              
  Lines       25203    25206       +3     
  Branches     6350     6350              
==========================================
- Hits        22212    22201      -11     
- Misses       2991     3005      +14     
Impacted Files Coverage Δ
packages/semi-foundation/form/foundation.ts 87.35% <100.00%> (+0.10%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cypress
Copy link

cypress bot commented Jul 18, 2023

1 failed tests on run #1767 ↗︎

1 211 10 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 6179b63 into 6ed4f82...
Project: semi-design Commit: 5be7b1f9bb ℹ️
Status: Failed Duration: 12:44 💡
Started: Jul 18, 2023 8:40 AM Ended: Jul 18, 2023 8:53 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@pointhalo pointhalo merged commit c3a9f82 into main Jul 18, 2023
9 of 10 checks passed
@pointhalo pointhalo deleted the fix-validateValues branch July 18, 2023 08:59
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.

None yet

2 participants