Fix: use a maintained minimal event emitter to prevent missing events error#129
Merged
jankapunkt merged 6 commits intodevfrom Sep 11, 2023
Merged
Fix: use a maintained minimal event emitter to prevent missing events error#129jankapunkt merged 6 commits intodevfrom
jankapunkt merged 6 commits intodevfrom
Conversation
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem description
The release 2.5.0 contains an unintended issue, where
eventsis implied as provider forEventEmitter.However this was due to an assumption that RN simply bundles node's
eventsas they don't contain any native bindings. However, this is not the case and theeventsthat 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'
NativeEventEmitterand wrapping it to contain theonandofffunctions to preserve the original API.Tests
During test environment it falls back to
eventswhich 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
eventsin yourdependencies' ordevDependenciesor any other dependencies (optional, peer) inpackage.json`Any issues reported are much appreciated!