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

internal: Remove Enzyme from visx #1893

Merged
merged 17 commits into from
Dec 18, 2024
Merged

Conversation

chazcb
Copy link
Collaborator

@chazcb chazcb commented Dec 17, 2024

This PR replaces Enzyme with RTL in prep for React 19 support.

🏠 Internal

  • Replaces Enzyme with RTL while avoiding jest.mock usage

@williaster williaster changed the title Remove Enzyme from visx internal: Remove Enzyme from visx Dec 17, 2024
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Thanks @chazcb , this is awesome 👏 🤖

Noticed some minor problems I can fix.

packages/visx-axis/test/Axis.test.tsx Outdated Show resolved Hide resolved
packages/visx-axis/test/Axis.test.tsx Outdated Show resolved Hide resolved
packages/visx-axis/test/AxisBottom.test.tsx Outdated Show resolved Hide resolved
packages/visx-axis/test/AxisLeft.test.tsx Outdated Show resolved Hide resolved
packages/visx-axis/test/AxisLeft.test.tsx Outdated Show resolved Hide resolved
packages/visx-shape/test/Pie.test.tsx Outdated Show resolved Hide resolved
Comment on lines +34 to +37
// Verify the path connects the correct points
const dAttribute = path?.getAttribute('d') || '';
expect(dAttribute).toContain('M100,100'); // Should start at x,y
expect(dAttribute).toContain('150,150'); // Should end at x+dx,y+dy
Copy link
Collaborator

Choose a reason for hiding this comment

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

improved the tests here! 🤖 🎉

packages/visx-tooltip/test/useTooltipInPortal.test.tsx Outdated Show resolved Hide resolved
packages/visx-tooltip/test/Tooltip.test.tsx Show resolved Hide resolved
@@ -40,8 +40,5 @@
},
"publishConfig": {
"access": "public"
},
"devDependencies": {
"@testing-library/react-hooks": "^8.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

left over from #1890, hoping this fixes the failing @visx/react-spring hooks test suite

Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

we did it finish 🏁

@williaster williaster merged commit 7c70f71 into master Dec 18, 2024
2 checks passed
@williaster williaster mentioned this pull request Dec 19, 2024
@darthmaim
Copy link
Contributor

darthmaim commented Dec 19, 2024

This is really amazing 🎉

If this was done using LLM this might warrant a blog post or so somewhere...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants