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: use a maintained minimal event emitter to prevent missing events error #129

Merged
merged 6 commits into from
Sep 11, 2023

Conversation

jankapunkt
Copy link
Member

Problem description

The release 2.5.0 contains an unintended issue, where events is implied as provider for EventEmitter.

However this was due to an assumption that RN simply bundles node's events as they don't contain any native bindings. However, this is not the case and the events that may have been used is actually a derivative from the npm registry:

https://www.npmjs.com/package/events

This is insufficient as it still requires an external dependency. Futhermore it raises errors on new installations: #116 (comment)

Solution

This PR attempts to solve this by using React Natives' NativeEventEmitter and wrapping it to contain the on and off functions to preserve the original API.

Tests

During test environment it falls back to events which is then supplied by Node, since we use Node to run the tests.

How to test in your setup?

You can simply install this via

$ npm install "git+ssh://git@github.com/meteorrn/meteor-react-native.git#fix-events"

and then build your app. Please make sure you have no events in your dependencies' or devDependenciesor any other dependencies (optional, peer) inpackage.json`

Any issues reported are much appreciated!

@jankapunkt jankapunkt changed the title Fix: use React Native NativeEventEmitter Fix: use a maintained minimal event emitter to prevent missing events error Sep 11, 2023
@jankapunkt jankapunkt merged commit 623295c into dev Sep 11, 2023
9 checks passed
@jankapunkt jankapunkt mentioned this pull request Sep 11, 2023
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.

1 participant