fix: devtools cannot be closed correctly

[Jack-Works]

Summary

Fix devtools cannot be shutdown by bridge.shutdown().

How did you test this change?

[Jack-Works]

$ node ./scripts/jest/jest-cli.js --build -r=stable --env=development --ci
node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module 'chalk'
Require stack:
- /home/circleci/project/scripts/jest/jest-cli.js

🤔?

[Jack-Works]

hello, @hoxyq can you take a look at this?

[hoxyq]

hello, @hoxyq can you take a look at this?

Looks good to me, I will debug it on Monday to check if there are some downsides, because we've been using bridge.send for a long time now, to notify all bridge's subscribers. We might use both here, though, need to double-check.

[Jack-Works]

hello, @hoxyq can you take a look at this?

Looks good to me, I will debug it on Monday to check if there are some downsides, because we've been using bridge.send for a long time now, to notify all bridge's subscribers. We might use both here, though, need to double-check.

It has been a long time for me so I forgot why I found this code is incorrect. It maybe caused by the confusion between send and emit.

[hoxyq]

hello, @hoxyq can you take a look at this?

Looks good to me, I will debug it on Monday to check if there are some downsides, because we've been using bridge.send for a long time now, to notify all bridge's subscribers. We might use both here, though, need to double-check.

It has been a long time for me so I forgot why I found this code is incorrect. It maybe caused by the confusion between send and emit.

This is really good catch, thanks for spotting this.

I can see that we are subscribing to shutdown event through bridge in different spots via bridge.addListener('shutdown', ...).

But with our current implementation, it looks like this event is never fired: I've ran the test that you have updated, it fails.

I believe here we should call both emit and send, because calling send is required to notify other entities, for example:

react/packages/react-devtools-extensions/src/backendManager.js

Lines 106 to 134 in ce6842d

	const bridge = new Bridge({
	listen(fn) {
	const listener = (event: $FlowFixMe) => {
	if (
	event.source !== window ||
	!event.data ||
	event.data.source !== 'react-devtools-content-script' ||
	!event.data.payload
	) {
	return;
	}
	fn(event.data.payload);
	};
	window.addEventListener('message', listener);
	return () => {
	window.removeEventListener('message', listener);
	};
	},
	send(event: string, payload: any, transferable?: Array<any>) {
	window.postMessage(
	{
	source: 'react-devtools-bridge',
	payload: {event, payload},
	},
	'*',
	transferable,
	);
	},
	});

[Jack-Works]

@hoxyq fixed!