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(react-native): import "node:events" lazily #1858

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

kettanaito
Copy link
Member

Changes

  • Adds a context on the SetupServerApi class.
  • Adds context.nodeEvents() getter that lazily imports node:events and returns undefined if the import failed (e.g. in React Native).
  • Moves the AbortSignal max listeners handling from the request listener to a private method of the SetupServerApi class. Makes it fail-fast.

private createContext(): SetupServerInternalContext {
return {
get nodeEvents() {
return import('node:events')
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this Promise should only ever fire once. I'm not sure if I'm structuring this correctly but the intention is to try to import this module once so each outgoing request (request listener) doesn't pend on this import, increasing the response time).

Copy link
Contributor

@mattcosta7 mattcosta7 left a comment

Choose a reason for hiding this comment

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

I think this makes sense, hopefully that helps work around the issues with this api in react-native like environments.

Re the import promise only being called once, I think it should resolve immediately since import calls are cached as far as I know, so that should be fine

@DogukanTansuk
Copy link

@mattcosta7 Can we release a beta version with this change? I could try and give feedback.

@kettanaito
Copy link
Member Author

@DogukanTansuk, you can always install a package from a particular branch/commit. I believe all modern package managers support that (NPM, Yarn, PNPM). You can install it that way and let me know if this fixes the issue? Thanks!

@kettanaito
Copy link
Member Author

I'm promoting this to a release candidate because the change is backward-compatible and has no effect on the existing msw/node integrations. Folks would have to give this a try for msw/native and report back whether the issue is fixed. That's the optimal course of action until we have an official React Native example we can run in the existing smoke tests pipeline.

@kettanaito kettanaito merged commit 0d79ec4 into main Nov 16, 2023
9 checks passed
@kettanaito kettanaito deleted the fix/react-native-events branch November 16, 2023 09:43
@kettanaito
Copy link
Member Author

Released: v2.0.7 🎉

This has been released in v2.0.7!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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.

Expo / React Native not having support for 'events'
3 participants