Skip to content

Conversation

@tmkx
Copy link
Contributor

@tmkx tmkx commented Apr 3, 2022

  1. Auto restore container to the original place
  2. Add a container option, so that we can avoid creating a new container every time

@vercel
Copy link

vercel bot commented Apr 3, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/util/EUQE1QgiStCkfdMRC1jgr4cFFxZv
✅ Preview: https://util-git-fork-tmkx-fix-portal-restore-react-component.vercel.app

});

useEffect(() => {
// Restore container to original place
Copy link
Member

Choose a reason for hiding this comment

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

Patch ref comment here in case other people want to know why:

https://reactjs.org/blog/2022/03/29/react-v18.html#new-strict-mode-behaviors

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

Update package.json with React 18 and add test case about this change.

@tmkx
Copy link
Contributor Author

tmkx commented Apr 6, 2022

Update package.json with React 18 and add test case about this change.

Upgrading to React 18 will cause a breaking change, but this works with 17.

@tmkx
Copy link
Contributor Author

tmkx commented Apr 6, 2022

or do we only update the version in devDependencies?

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

Update package.json with React 18 and add test case about this change.

Upgrading to React 18 will cause a breaking change, but this works with 17.

devDeps not peerDeps~

@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #288 (0e70c46) into master (43cf427) will increase coverage by 0.23%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
+ Coverage   63.74%   63.97%   +0.23%     
==========================================
  Files          44       44              
  Lines         935      941       +6     
  Branches      327      330       +3     
==========================================
+ Hits          596      602       +6     
  Misses        301      301              
  Partials       38       38              
Impacted Files Coverage Δ
src/Portal.tsx 95.45% <100.00%> (+1.70%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@tmkx
Copy link
Contributor Author

tmkx commented Apr 6, 2022

It seems that enzyme does not works fine with React 18.
Have you considered moving to @testing-library/react ?

see enzymejs/enzyme#2429 (comment)

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

It seems that enzyme does not works fine with React 18. Have you considered moving to @testing-library/react ?

see enzymejs/enzyme#2429 (comment)

Yes. antd has part moved: ant-design/ant-design#34787

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

Let me handle this migration.

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

#289 Merged

@zombieJ
Copy link
Member

zombieJ commented Apr 6, 2022

https://github.com/react-component/util/pull/288/files#r843545160

Patch a comment on the effect to tell why need this~

@zombieJ zombieJ merged commit f5c30d8 into react-component:master Apr 7, 2022
@tmkx tmkx deleted the fix/portal-restore branch April 7, 2022 06:15
@zombieJ
Copy link
Member

zombieJ commented Apr 7, 2022

+ rc-util@5.19.4

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.

2 participants