dotintent / react-native-ble-plx

React Native BLE library
Apache License 2.0
2.97k stars 498 forks source link

🐛 Making methods async and/or arrow functions in v3.2.0 is a breaking change #1208

Open josmithua opened 1 month ago

josmithua commented 1 month ago

Prerequisites

Expected Behavior

v3.2.0 should have been v4.0.0 (assuming the project is following semantic versioning).

Current Behavior

Updating to v3.2.0 breaks typescript checking, return values, tests, etc.

Library version

v3.2.0

Device

any

Environment info

n/a

Steps to reproduce

Update to v3.2.0 Run tsc on project codebase using this library.

Formatted code sample or link to a repository

n/a

Relevant log output

n/a

Additional information

No response

josmithua commented 1 month ago

There is also a console.log statement in onStateChange https://github.com/dotintent/react-native-ble-plx/blob/2c8c13891ad154aadc803f880f4e5a3af0476b87/src/BleManager.js#L313-L314

josmithua commented 1 month ago

Furthermore, making all the instance methods of BleManager arrow functions breaks tests that automock the BleManager class. See the note here: https://jestjs.io/docs/es6-class-mocks#automatic-mock

If I manually edit the BleManager.js in my node_modules and change the methods back to regular non-arrow methods, my tests work fine.

The following MRE breaks with v3.2.0:

import { bleManager } from "./ble";
jest.mock("react-native-ble-plx");

describe("bleManager", () => {
  it("stopDeviceScan", async () => {
    // Fails on the following line with:
    // Error: Property `stopDeviceScan` does not exist in the provided object
    const spy = jest.spyOn(bleManager, "stopDeviceScan").mockResolvedValue();

    await bleManager.stopDeviceScan();

    expect(spy).toHaveBeenCalled();
  });
});

Works fine with stopDeviceScan as non arrow function (in v3.1.0)

josmithua commented 1 month ago

I would please request that you change the arrow functions back to methods so that they are on the BleManager prototype, and bump the version to v4. For now I will be downgrading to 3.1.0. Cheers

madej10 commented 1 month ago

@josmithua thanks for letting us know. Looking into it