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

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

Closed
2 tasks done
josmithua opened this issue Jun 1, 2024 · 4 comments · Fixed by #1225
Closed
2 tasks done

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

josmithua opened this issue Jun 1, 2024 · 4 comments · Fixed by #1225
Labels

Comments

@josmithua
Copy link
Contributor

Prerequisites

  • I checked the documentation and FAQ without finding a solution
  • I checked to make sure that this issue has not already been filed

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 josmithua added the bug label Jun 1, 2024
@josmithua
Copy link
Contributor Author

There is also a console.log statement in onStateChange

console.log(currentState)
if (!cancelled) {

@josmithua
Copy link
Contributor Author

josmithua commented Jun 1, 2024

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 josmithua changed the title 🐛 Making startDeviceScan/stopDeviceScan async in v3.2.0 is a breaking change 🐛 Making methods async and/or arrow functions in v3.2.0 is a breaking change Jun 1, 2024
@josmithua
Copy link
Contributor Author

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
Copy link

madej10 commented Jun 4, 2024

@josmithua thanks for letting us know. Looking into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants