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

feat: Add support for custom EventEmitter #1999

Merged
merged 6 commits into from
Sep 3, 2023

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Aug 25, 2023

Pull Request

Issue

A lot of JS frameworks that uses Parse throws errors when trying to use event emitter.

 Uncaught TypeError: Super expression must either be null or a function
    _inherits Babel
    Subscription LiveQuerySubscription.js:149

Closes: #1412

Approach

Parse.CoreManager.setEventEmitter(MY_EMITTER_CLASS);
Parse.initialize(...)
  • Decouple EventEmitter from LiveQuery classes
  • Decouple LiveQuery from LiveQueryClient
  • Run test suite against react-native
  • Prevent overriding the custom event emitter

Alternative

We could write our own EventEmitter in vanilla instead of node:events and polyfilling

Tasks

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

@parse-github-assistant
Copy link

Thanks for opening this pull request!

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (be0c8a6) 100.00% compared to head (2a3d339) 100.00%.
Report is 1 commits behind head on alpha.

❗ Current head 2a3d339 differs from pull request most recent head 43e8e7d. Consider uploading reports for the commit 43e8e7d to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             alpha     #1999   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           61        61           
  Lines         6141      6168   +27     
  Branches      1498      1499    +1     
=========================================
+ Hits          6141      6168   +27     
Files Changed Coverage Δ
src/CoreManager.js 100.00% <100.00%> (ø)
src/EventEmitter.js 100.00% <100.00%> (ø)
src/LiveQueryClient.js 100.00% <100.00%> (ø)
src/LiveQuerySubscription.js 100.00% <100.00%> (ø)
src/Parse.ts 100.00% <100.00%> (ø)
src/ParseLiveQuery.js 100.00% <100.00%> (ø)

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

@dplewis dplewis requested review from a team and mtrezza August 25, 2023 17:23
@mtrezza mtrezza changed the title feat: Allow for custom EventEmitter feat: Add support for custom EventEmitter Aug 27, 2023
@mtrezza mtrezza mentioned this pull request Aug 27, 2023
4 tasks
@dplewis
Copy link
Member Author

dplewis commented Aug 29, 2023

@mtrezza This a new feature feat: but the side effects fixes a lot of issues. This forces a change in the import order. For instance, If you loaded LiveQuery before EventEmitter the SDK breaks. I would personally like to use eventemitter3 for performance or create my own. This PR lays the groundwork for an isomorphic SDK

@mtrezza
Copy link
Member

mtrezza commented Aug 30, 2023

Do you mean this is a breaking change?

@dplewis
Copy link
Member Author

dplewis commented Aug 30, 2023

Functionality is still the same but the new order allows the developer to control the flow. This shouldn’t be a breaking change. Moving extend an unloaded class to the contructor makes this PR possible.

@mtrezza
Copy link
Member

mtrezza commented Aug 30, 2023

I want to make sure I understand this correctly, you wrote:

This forces a change in the import order. For instance, If you loaded LiveQuery before EventEmitter the SDK breaks.

Do you mean that currently (pre-PR) if a developer loaded LiveQuery before EventEmitter the SDK crashes, but with this PR that is not the case anymore?

@dplewis
Copy link
Member Author

dplewis commented Aug 30, 2023

We don’t support ES6. The many JS frameworks that uses Parse rearrange the imports. Unless they use the minimized version, we have no control meaning developers don’t have control.

@mtrezza
Copy link
Member

mtrezza commented Aug 30, 2023

Alright, if it's not a breaking change, we may just leave this open for a bit longer in case anyone from @parse-community/server has a comment on this. Otherwise we go ahead an merge.

@mtrezza mtrezza merged commit ca568a6 into parse-community:alpha Sep 3, 2023
6 checks passed
parseplatformorg pushed a commit that referenced this pull request Sep 3, 2023
# [4.2.0-alpha.10](4.2.0-alpha.9...4.2.0-alpha.10) (2023-09-03)

### Features

* Add support for custom EventEmitter ([#1999](#1999)) ([ca568a6](ca568a6))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.2.0-alpha.10

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Sep 3, 2023
parseplatformorg pushed a commit that referenced this pull request Sep 16, 2023
# [4.3.0-beta.1](4.2.0...4.3.0-beta.1) (2023-09-16)

### Bug Fixes

* `ParseUser.linkWith` doesn't remove anonymous auth data ([#2007](#2007)) ([7e2585c](7e2585c))
* Hard-coding of `react-native` path does not work for workspace builds ([#1930](#1930)) ([8222f3c](8222f3c))

### Features

* Add Bytes type to `Parse.Schema` ([#2001](#2001)) ([343d0d7](343d0d7))
* Add Cloud Code context accessibility to `ParseUser.logIn` ([#2010](#2010)) ([2446007](2446007))
* Add support for custom EventEmitter ([#1999](#1999)) ([ca568a6](ca568a6))
* Add support for excluding keys in `ParseQuery.findAll` ([#2000](#2000)) ([012ba4c](012ba4c))
* Add support to invoke a Cloud Function with a custom `installationId` via `Parse.Cloud.run` ([#1939](#1939)) ([eb70b93](eb70b93))
* Allow overriding `Parse.Error` message with custom message via new Core Manager option `PARSE_ERRORS` ([#2014](#2014)) ([be0c8a6](be0c8a6))
* Login with username, password and additional authentication data via `ParseUser.logInWithAdditionalAuth` ([#1955](#1955)) ([2bad411](2bad411))
@parseplatformorg
Copy link
Contributor

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

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Sep 16, 2023
parseplatformorg pushed a commit that referenced this pull request Sep 26, 2023
# [4.3.0-alpha.1](4.2.0...4.3.0-alpha.1) (2023-09-26)

### Bug Fixes

* `ParseUser.linkWith` doesn't remove anonymous auth data ([#2007](#2007)) ([7e2585c](7e2585c))
* Hard-coding of `react-native` path does not work for workspace builds ([#1930](#1930)) ([8222f3c](8222f3c))

### Features

* Add Bytes type to `Parse.Schema` ([#2001](#2001)) ([343d0d7](343d0d7))
* Add Cloud Code context accessibility to `ParseUser.logIn` ([#2010](#2010)) ([2446007](2446007))
* Add support for custom EventEmitter ([#1999](#1999)) ([ca568a6](ca568a6))
* Add support for excluding keys in `ParseQuery.findAll` ([#2000](#2000)) ([012ba4c](012ba4c))
* Add support to invoke a Cloud Function with a custom `installationId` via `Parse.Cloud.run` ([#1939](#1939)) ([eb70b93](eb70b93))
* Allow overriding `Parse.Error` message with custom message via new Core Manager option `PARSE_ERRORS` ([#2014](#2014)) ([be0c8a6](be0c8a6))
* Login with username, password and additional authentication data via `ParseUser.logInWithAdditionalAuth` ([#1955](#1955)) ([2bad411](2bad411))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.3.0-alpha.1

parseplatformorg pushed a commit that referenced this pull request Nov 16, 2023
# [4.3.0](4.2.0...4.3.0) (2023-11-16)

### Bug Fixes

* `ParseUser.linkWith` doesn't remove anonymous auth data ([#2007](#2007)) ([7e2585c](7e2585c))
* Hard-coding of `react-native` path does not work for workspace builds ([#1930](#1930)) ([8222f3c](8222f3c))

### Features

* Add Bytes type to `Parse.Schema` ([#2001](#2001)) ([343d0d7](343d0d7))
* Add Cloud Code context accessibility to `ParseUser.logIn` ([#2010](#2010)) ([2446007](2446007))
* Add support for custom EventEmitter ([#1999](#1999)) ([ca568a6](ca568a6))
* Add support for excluding keys in `ParseQuery.findAll` ([#2000](#2000)) ([012ba4c](012ba4c))
* Add support to invoke a Cloud Function with a custom `installationId` via `Parse.Cloud.run` ([#1939](#1939)) ([eb70b93](eb70b93))
* Allow overriding `Parse.Error` message with custom message via new Core Manager option `PARSE_ERRORS` ([#2014](#2014)) ([be0c8a6](be0c8a6))
* Login with username, password and additional authentication data via `ParseUser.logInWithAdditionalAuth` ([#1955](#1955)) ([2bad411](2bad411))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.3.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Nov 16, 2023
martinpfannemueller pushed a commit to martinpfannemueller/Parse-SDK-JS that referenced this pull request Apr 13, 2024
…)"

This reverts commit ca568a6.

# Conflicts:
#	src/Parse.ts
@dplewis dplewis deleted the event-emitter branch April 13, 2024 13:40
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.

ReactNative - EventEmitter bug
3 participants