Cannot get websocket.send because property send [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

[FezVrasta]

Flow version: 0.153.0

Expected behavior

Flow shouldn't throw a method-unbinding error.

Actual behavior

//@flow
const websocket = new WebSocket('ws://localhost:8080');

expect(websocket.send).toHaveBeenCalled();

This code throws a method-unbinding error on websocket.send:

Cannot get websocket.send because property send [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

[pascalduez]

I afraid we have the same problem with any of the core and flow-typed definitions using classes.

Another example:

// @flow

let elem = document.querySelector('input');

if (elem) {
  typeof elem.setCustomValidity === 'function';
}

Cannot get `one.setCustomValidity` because  property `setCustomValidity` [1] cannot be unbound from the  context [2] where it was defined.Flow(method-unbinding)
utils.js(6, 14): [1] property `setCustomValidity`
dom.js(3517, 3): [2] context

[dsainati1]

For the example with elem.setCustomValidity why do you need to check its type? The return of document.querySelector('input') is defined as HTMLInputElement | null so you should only need to check whether elem is null here. Or are you using a flow-typed library with different type definitions?

For the jest example, Flow is unfortunately not able to detect that this specific instance of method unbinding is "safe" because in general passing an unbound method to a function is not going to be safe: the function may do arbitrary things with it. I would recommend suppressing this error for now.

[FezVrasta]

For the example with elem.setCustomValidity why do you need to check its type? The return of document.querySelector('input') is defined as HTMLInputElement | null so you should only need to check whether elem is null here. Or are you using a flow-typed library with different type definitions?

Maybe to support older browsers?

For the jest example, Flow is unfortunately not able to detect that this specific instance of method unbinding is "safe" because in general passing an unbound method to a function is not going to be safe: the function may do arbitrary things with it. I would recommend suppressing this error for now.

That's unfortunate, I wonder if at Facebook this kind of tests is not used? I can't see how they could have missed such an issue otherwise.

[pascalduez]

For the example with elem.setCustomValidity why do you need to check its type? The return of document.querySelector('input') is defined as HTMLInputElement | null so you should only need to check whether elem is null here. Or are you using a flow-typed library with different type definitions?

Maybe to support older browsers?

Yes exactly, I stripped down the sample to the bar minimum.

Other example in this vein:

if (typeof Array.prototype.flatMap !== 'function') { ... }

[charlag]

This does not work either currently:

typeof navigator.registerProtocolHandler === "function"

which I find a bit silly. Even if I would not be doing typeof but would just check it as boolean it should be okay.

[shirohana]

Here's a case that I expect both should be work:

const arr = []

// works
const flattedArr1 = [].concat(...arr)

// can't works
const flattedArr2 = [].concat.apply(arr)

// can't works
const flattedArr3 = Array.prototype.concat.apply([], arr)

[FezVrasta]

To add to the list, useQuery + refetch from @apollo/client fails too:

const { refetch } = useQuery(something);

The above throws a method-unbinding on refetch

[pascalduez]

Another case:

import memoizeOne from 'memoize-one'

memoizeOne(JSON.parse);

[gkz]

@dsainati1 I think a lot of these cases could be handled by updating our lib defs to use object types instead of classes.

E.g. There is not reason why JSON should be defined as a class in our lib defs https://github.com/facebook/flow/blob/master/lib/core.js#L1598

[dsainati1]

This is on our radar but at the moment it isn't a priority. We are working on a large restructuring of Flow's type inference, and one of the side effects of this work is that it will make special-case behavior like this much easier, but at the moment it is challenging to make ad-hoc adjustments like this. This will get fixed eventually, but it is unlikely that I will get to it soon.

[FezVrasta]

Meanwhile is there any way to disable this rule globally?

[dsainati1]

I would recommend just suppressing it where it occurs. It is safe to do so because error suppressions using the method-unbinding code will not catch any errors other than this particular kind.

[geraintwhite]

I would recommend just suppressing it where it occurs. It is safe to do so because error suppressions using the method-unbinding code will not catch any errors other than this particular kind.

If a project has 500 such occurrences of the method-unbinding error (mainly in unit tests such as the OP's), is there no way of disabling this rule globally?

[gkz]

There is no way to disable it globally (nor can there be, it is required to safely type this in general). What are you using for your unit test framework? Jest?

[geraintwhite]

There is no way to disable it globally (nor can there be, it is required to safely type this in general). What are you using for your unit test framework? Jest?

Yes, I use jest in my react-native project.

I'm updating flow from 0.149 to 0.158 as part of a react-native upgrade.

The vast majority of the new errors come from my usage of an APIConnector class and using jest to assert method calls.

I also get method-unbinding errors when using jest to assert method calls on classes exported from react-native.

Cannot get `DeviceEventEmitter.emit` because property `emit` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(...)


Cannot get `Linking.openURL` because property `openURL` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(Linking.openURL).toHaveBeenCalledWith(...)


Cannot get `Linking.openSettings` because property `openSettings` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(Linking.openSettings).toHaveBeenCalled();


Cannot get `Keyboard.dismiss` because property `dismiss` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(Keyboard.dismiss).toHaveBeenCalled();


Cannot get `AppState.addEventListener` because property `addEventListener` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(AppState.addEventListener).toHaveBeenCalledWith(...)


Cannot get `AppState.removeEventListener` because property `removeEventListener` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(AppState.removeEventListener).toHaveBeenCalledWith(...)


Cannot get `apiConnector.getSiteUsers` because property `getSiteUsers` [1] cannot be unbound from the context [2] where it was defined. [method-unbinding]

expect(apiConnector.getSiteUsers).toHaveBeenCalledWith(...);

[gkz]

OK, thanks for the use-cases. We have the same issue as well, with method-unbinding and mocking of methods in Jest. You can just suppress the errors for now (// $FlowFixMe[method-unbinding] only suppresses that error, so does not create a general safety issue), but since this is an issue we're having as well it's something we're thinking about how to deal with in the long term.