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: EventEmitter warnings #1615

Merged
merged 4 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ build(turf): update to version 6.5.0 ([#1638](https://github.com/react-native-ma
ci: two scripts for linting with and without fix ([#1630](https://github.com/react-native-mapbox-gl/maps/pull/1630))
feat(Camera) add an optional `allowUpdates` boolean prop ([#1619](https://github.com/react-native-mapbox-gl/maps/pull/1619))
refactor(example): remove unused modules and scripts ([#1618](https://github.com/react-native-mapbox-gl/maps/pull/1618))
fix(react-native): update api to get rid of EventEmitter warnings ([#1615](https://github.com/react-native-mapbox-gl/maps/pull/1615))
fix(Camera) persist zoom when changing from `bounds` to `centerCoordinate`, fix zero padding not causing map to update, create unified example showcasing bounds/centerCoordinate/zoom/padding ([#1614](https://github.com/react-native-mapbox-gl/maps/pull/1614))
Update MapLibre to 5.12.1 on iOS ([#1596](https://github.com/react-native-mapbox-gl/maps/pull/1596))
Update ShapeSource methods to make it usable with any cluster ( Use cluster itself instead of cluster_id as first argument for getClusterExpansionZoom/getClusterLeaves/getClusterChildren methods. Version < 9 methods still supports passing cluster_id as a first argument but a deprecation warning will be shown. ) ([#1499](https://github.com/react-native-mapbox-gl/maps/pull/1499))
Expand Down
1 change: 0 additions & 1 deletion __tests__/__mocks__/react-native.mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ jest.mock('react-native/Libraries/Image/resolveAssetSource', () => {
jest.mock('NativeEventEmitter', () => {
function MockEventEmitter() {}
MockEventEmitter.prototype.addListener = jest.fn(() => ({remove: jest.fn()}));
MockEventEmitter.prototype.removeListener = jest.fn();
return MockEventEmitter;
});
13 changes: 2 additions & 11 deletions __tests__/modules/location/locationManager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,19 +198,14 @@ describe('LocationManager', () => {

// native location manager has no #stop exposed in tests?
MapboxGLLocationManager.stop = jest.fn();
jest.spyOn(LocationModuleEventEmitter, 'removeListener');

MapboxGL.LocationCallbackName = {Update: 'MapboxUserLocationUpdate'};

expect(locationManager._isListening).toStrictEqual(true);

locationManager.stop();

expect(MapboxGLLocationManager.stop).toHaveBeenCalledTimes(1);
expect(LocationModuleEventEmitter.removeListener).toHaveBeenCalledWith(
MapboxGL.LocationCallbackName.Update,
locationManager.onUpdate,
);
expect(locationManager.subscription.remove).toHaveBeenCalled();

expect(locationManager._isListening).toStrictEqual(false);
});
Expand All @@ -221,18 +216,14 @@ describe('LocationManager', () => {

// native location manager has no #stop exposed in tests?
MapboxGLLocationManager.stop = jest.fn();
jest.spyOn(LocationModuleEventEmitter, 'removeListener');

MapboxGL.LocationCallbackName = {Update: 'MapboxUserLocationUpdate'};

expect(locationManager._isListening).toStrictEqual(false);

locationManager.stop();

expect(MapboxGLLocationManager.stop).toHaveBeenCalledTimes(1);
expect(
LocationModuleEventEmitter.removeListener,
).not.toHaveBeenCalled();
expect(locationManager.subscription.remove).not.toHaveBeenCalled();
});
});

Expand Down
13 changes: 9 additions & 4 deletions __tests__/modules/offline/offlineManager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('offlineManager', () => {
const noop = () => {};
await MapboxGL.offlineManager.createPack(packOptions, noop, noop);
expect(spy).toHaveBeenCalledTimes(2);
spy.mockRestore();
spy.mockClear();
});

it('should call progress listener', async () => {
Expand Down Expand Up @@ -133,12 +133,17 @@ describe('offlineManager', () => {
});

it('should unsubscribe from native events', async () => {
const spy = jest.spyOn(OfflineModuleEventEmitter, 'removeListener');
const noop = () => {};

await MapboxGL.offlineManager.createPack(packOptions, noop, noop);
MapboxGL.offlineManager.unsubscribe(packOptions.name);
expect(spy).toHaveBeenCalledTimes(2);
spy.mockRestore();

expect(
MapboxGL.offlineManager.subscriptionProgress.remove,
).toHaveBeenCalledTimes(1);
expect(
MapboxGL.offlineManager.subscriptionError.remove,
).toHaveBeenCalledTimes(1);
});

it('should unsubscribe event listeners once a pack download has completed', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ public void onFailure(@NonNull Exception exception) {
);
}

@ReactMethod
public void addListener(String eventName) {
// Set up any upstream listeners or background tasks as necessary
}

@ReactMethod
public void removeListeners(Integer count) {
// Remove upstream listeners, stop unnecessary background tasks
}

private void startLocationManager() {
locationManager.addLocationListener(onUserLocationChangeCallback);
locationManager.setMinDisplacement(mMinDisplacement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ public void setLogLevel(String level) {
Logger.setVerbosity(logLevel);
}

@ReactMethod
public void addListener(String eventName) {
// Set up any upstream listeners or background tasks as necessary
}

@ReactMethod
public void removeListeners(Integer count) {
// Remove upstream listeners, stop unnecessary background tasks
}

public void onLog(String level, String tag, String msg, Throwable tr) {
WritableMap event = Arguments.createMap();
event.putString("message", msg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ public String getName () {
return REACT_CLASS;
}

@ReactMethod
public void addListener(String eventName) {
// Set up any upstream listeners or background tasks as necessary
}

@ReactMethod
public void removeListeners(Integer count) {
// Remove upstream listeners, stop unnecessary background tasks
}

@ReactMethod
public void createPack(ReadableMap options, final Promise promise) {
final String name = ConvertUtils.getString("name", options, "");
Expand Down
8 changes: 3 additions & 5 deletions javascript/modules/location/locationManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class LocationManager {
this._lastKnownLocation = null;
this._isListening = false;
this.onUpdate = this.onUpdate.bind(this);
this.subscription = null;
}

async getLastKnownLocation() {
Expand Down Expand Up @@ -67,7 +68,7 @@ class LocationManager {
if (!this._isListening) {
MapboxGLLocationManager.start(displacement);

LocationModuleEventEmitter.addListener(
this.subscription = LocationModuleEventEmitter.addListener(
MapboxGL.LocationCallbackName.Update,
this.onUpdate,
);
Expand All @@ -80,10 +81,7 @@ class LocationManager {
MapboxGLLocationManager.stop();

if (this._isListening) {
LocationModuleEventEmitter.removeListener(
MapboxGL.LocationCallbackName.Update,
this.onUpdate,
);
this.subscription.remove();
}

this._isListening = false;
Expand Down
27 changes: 15 additions & 12 deletions javascript/modules/offline/offlineManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class OfflineManager {

this._onProgress = this._onProgress.bind(this);
this._onError = this._onError.bind(this);

this.subscriptionProgress = null;
this.subscriptionError = null;
}

/**
Expand Down Expand Up @@ -261,7 +264,7 @@ class OfflineManager {
const totalProgressListeners = Object.keys(this._progressListeners).length;
if (isFunction(progressListener)) {
if (totalProgressListeners === 0) {
OfflineModuleEventEmitter.addListener(
this.subscriptionProgress = OfflineModuleEventEmitter.addListener(
MapboxGL.OfflineCallbackName.Progress,
this._onProgress,
);
Expand All @@ -272,7 +275,7 @@ class OfflineManager {
const totalErrorListeners = Object.keys(this._errorListeners).length;
if (isFunction(errorListener)) {
if (totalErrorListeners === 0) {
OfflineModuleEventEmitter.addListener(
this.subscriptionError = OfflineModuleEventEmitter.addListener(
MapboxGL.OfflineCallbackName.Error,
this._onError,
);
Expand Down Expand Up @@ -306,18 +309,18 @@ class OfflineManager {
delete this._progressListeners[packName];
delete this._errorListeners[packName];

if (Object.keys(this._progressListeners).length === 0) {
OfflineModuleEventEmitter.removeListener(
MapboxGL.OfflineCallbackName.Progress,
this._onProgress,
);
if (
Object.keys(this._progressListeners).length === 0 &&
this.subscriptionProgress
) {
this.subscriptionProgress.remove();
}

if (Object.keys(this._errorListeners).length === 0) {
OfflineModuleEventEmitter.removeListener(
MapboxGL.OfflineCallbackName.Error,
this._onError,
);
if (
Object.keys(this._errorListeners).length === 0 &&
this.subscriptionError
) {
this.subscriptionError.remove();
}
}

Expand Down