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

Expand peer dep range of test utils to support RTL 14 #6264

Merged
merged 2 commits into from Apr 29, 2024
Merged

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Apr 24, 2024

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

@rspbot
Copy link

rspbot commented Apr 25, 2024

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

@LFDanLu
Copy link
Member Author

LFDanLu commented Apr 26, 2024

13 vs 14 of RTL certainly has breaking change, but the actual util functions right now don't have anything specific to 14 within them (only has fireEvent and pointerEventMap, the latter of which is only a object that a user can choose to import/not import for the purpose of providing to their own user setup aka someone on RTL 13 wouldn't use that)

@snowystinger
Copy link
Member

We don't make use of any of the things which were breaking?
Sorry, normally I'd check myself and verify. I'll try to find time to.

@LFDanLu
Copy link
Member Author

LFDanLu commented Apr 26, 2024

NP, so the test utils right now is just the stuff in react-aria/test-utils and react-spectrum/test-util, and the only things that use stuff from testing library is https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/test-utils/src/events.ts and https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/test-utils/src/userEventMaps.ts. The first is a fireEvent util so that isn't affected by user-event's breaking changes. The second is a userEventMap that is specific to 14, but is up to the user if they need it or not.

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Approving

I'm still worried that we can't support both once we start expanding into component specific utilities, but I'm ok with it so long as we're pre-releasing this

@LFDanLu
Copy link
Member Author

LFDanLu commented Apr 29, 2024

I'm still worried that we can't support both once we start expanding into component specific utilities, but I'm ok with it so long as we're pre-releasing this

I agree, it might turn that the component specific utils force RTL 14 but I def want to explore supporting sub-14 RTL at the same time as 14 in my experimental branch.

@rspbot
Copy link

rspbot commented Apr 29, 2024

@rspbot
Copy link

rspbot commented Apr 29, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@LFDanLu LFDanLu merged commit 233b650 into main Apr 29, 2024
25 checks passed
@LFDanLu LFDanLu deleted the bump_peers branch April 29, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants