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: Live Query not working on Expo React Native #2109

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Apr 13, 2024

Pull Request

Issue

#1999 introduced swappable custom EventEmitters for runtimes that didn't include Node's builtin EventEmitter or React-Native.

Fixed

start.js:39 Uncaught (in promise) TypeError: Class extends value undefined is not a constructor or null
    at node_modules/parse/lib/node/LiveQuerySubscription.js (LiveQuerySubscription.js:100:42)

The LiveQuery modules no longer extends from the EventEmitter (would break if EM doesn't exist) but copies on and emit from the prototype chain in the constructor where EM could exist. RN: Use a Private Property in EventEmitter created a private property in the addListener (copied to on). This breaks RN since we are trying to access a private property from a non-instance class (no longer extends so not an instance of EventEmitter therefore no access)

TypeError: attempted to use private field on non-instance

Closes: #2082, #2081, #1766, #1522

Approach

Instead of inheriting from the EventEmitter just use a wrapper.

This has been tested on

  "dependencies": {
    "@expo/metro-runtime": "3.1.3",
    "@react-native-async-storage/async-storage": "1.21.0",
    "expo": "50.0.15",
    "expo-status-bar": "1.11.1",
    "parse": "5.0.0",
    "react": "18.2.0",
    "react-dom": "18.2.0",
    "react-native": "0.73.6",
    "react-native-get-random-values": "1.11.0",
    "react-native-web": "0.19.6"
  },

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)

Copy link

Thanks for opening this pull request!

Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (72bc9ac) to head (c4d4d24).
Report is 8 commits behind head on alpha.

❗ Current head c4d4d24 differs from pull request most recent head 665911e. Consider uploading reports for the commit 665911e to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##            alpha     #2109      +/-   ##
===========================================
+ Coverage   99.98%   100.00%   +0.01%     
===========================================
  Files          61        64       +3     
  Lines        6185      6196      +11     
  Branches     1499      1500       +1     
===========================================
+ Hits         6184      6196      +12     
+ Misses          1         0       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dplewis dplewis requested review from mtrezza and a team April 13, 2024 23:11
@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2024

@mtrezza This has been tested by @martinpfannemueller #2081 (comment)

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

So this PR can be closed without merging?

@dplewis
Copy link
Member Author

dplewis commented Apr 15, 2024

So this PR can be closed without merging?

This can be merged. This PR fixes the issue posted and has been tested by multiple developers using Expo / React-Native

@mtrezza
Copy link
Member

mtrezza commented Apr 15, 2024

If it fixes an issue, could you add a test for the issue?

@dplewis
Copy link
Member Author

dplewis commented Apr 15, 2024

@mtrezza I added a test, looks like codecov is broken again.

@dplewis dplewis requested a review from mtrezza April 15, 2024 14:39
@mtrezza
Copy link
Member

mtrezza commented Apr 15, 2024

I believe what you are trying to say is that it's too much effort to write a test for it? So we'll merge without test.

Edit: Thanks for adding a test.

@mtrezza mtrezza merged commit 7a89665 into parse-community:alpha Apr 15, 2024
5 of 7 checks passed
parseplatformorg pushed a commit that referenced this pull request Apr 15, 2024
# [5.1.0-alpha.4](5.1.0-alpha.3...5.1.0-alpha.4) (2024-04-15)

### Bug Fixes

* Live Query not working on Expo React Native ([#2109](#2109)) ([7a89665](7a89665))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.1.0-alpha.4

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Apr 15, 2024
@dplewis dplewis deleted the expo-live-query branch April 17, 2024 16:40
parseplatformorg pushed a commit that referenced this pull request May 16, 2024
# [5.1.0-beta.1](5.0.0...5.1.0-beta.1) (2024-05-16)

### Bug Fixes

* `Parse.GeoPoint.current` returns `undefined` ([#2127](#2127)) ([3860535](3860535))
* Chrome browser console warning about unsafe header `access-control-expose-headers` when calling Cloud Function ([#2095](#2095)) ([7b73c03](7b73c03))
* Live Query not working on Expo React Native ([#2109](#2109)) ([7a89665](7a89665))
* Local datastore throws error when `Parse.Query.notEqualTo` is set to `null` ([#2102](#2102)) ([6afd32a](6afd32a))
* Multiple object updates of nested keys overwrite each other ([#1451](#1451)) ([fa4341a](fa4341a))
* Pending updates to nested field causes `ParseObject.toJSON()` to return incorrect object ([#1453](#1453)) ([23cc573](23cc573))
* Remove circular dependencies ([#2125](#2125)) ([b415165](b415165))

### Features

* Add password validation for user with unverified email via `Parse.User.verifyPassword` using master key and option `ignoreEmailVerification: true` ([#2076](#2076)) ([b0adf7e](b0adf7e))
* Add support for setting `Parse.ACL` from json ([#2097](#2097)) ([72bc9ac](72bc9ac))
* Allow setting custom queue for handling offline operations via `Parse.EventuallyQueue` ([#2106](#2106)) ([f92e4d4](f92e4d4))
* Improve installation object `Parse.Installation.currentInstallation` to support web push notifications ([#2119](#2119)) ([4fc62ce](4fc62ce))
* Lazy load `Parse.CoreManager` controllers to add support for swappable `CryptoController`, `LocalDatastoreController`, `StorageController`, `WebSocketController`, `ParseLiveQuery` ([#2100](#2100)) ([fbd0ab1](fbd0ab1))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.1.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label May 16, 2024
parseplatformorg pushed a commit that referenced this pull request May 16, 2024
# [5.1.0](5.0.0...5.1.0) (2024-05-16)

### Bug Fixes

* `Parse.GeoPoint.current` returns `undefined` ([#2127](#2127)) ([3860535](3860535))
* Chrome browser console warning about unsafe header `access-control-expose-headers` when calling Cloud Function ([#2095](#2095)) ([7b73c03](7b73c03))
* Live Query not working on Expo React Native ([#2109](#2109)) ([7a89665](7a89665))
* Local datastore throws error when `Parse.Query.notEqualTo` is set to `null` ([#2102](#2102)) ([6afd32a](6afd32a))
* Multiple object updates of nested keys overwrite each other ([#1451](#1451)) ([fa4341a](fa4341a))
* Pending updates to nested field causes `ParseObject.toJSON()` to return incorrect object ([#1453](#1453)) ([23cc573](23cc573))
* Remove circular dependencies ([#2125](#2125)) ([b415165](b415165))

### Features

* Add password validation for user with unverified email via `Parse.User.verifyPassword` using master key and option `ignoreEmailVerification: true` ([#2076](#2076)) ([b0adf7e](b0adf7e))
* Add support for setting `Parse.ACL` from json ([#2097](#2097)) ([72bc9ac](72bc9ac))
* Allow setting custom queue for handling offline operations via `Parse.EventuallyQueue` ([#2106](#2106)) ([f92e4d4](f92e4d4))
* Improve installation object `Parse.Installation.currentInstallation` to support web push notifications ([#2119](#2119)) ([4fc62ce](4fc62ce))
* Lazy load `Parse.CoreManager` controllers to add support for swappable `CryptoController`, `LocalDatastoreController`, `StorageController`, `WebSocketController`, `ParseLiveQuery` ([#2100](#2100)) ([fbd0ab1](fbd0ab1))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expo RN and ParseLiveQuery EventEmitter error
3 participants