Remove check for sender on channel.

[Gongreg]

Issue: Because of check for sender in channels library, it is impossible to communicate between manager and addons.

Right now it is only possible to communicate between

• addons -> provider
• provider -> manager.

If you want to communicate between manager and addons, you need to send the message from manager to provider and from provider to addons (and vice versa).

What I am afraid about is that I see that this check was added as a separate commit. Could we try to reach @mnmtanish so he could share his knowledge about why he added it in first place?

Also I would really like to get your opinion about this. I thought about adding this check as an opt-out, but @ndelangen feels like we remove the check completely if possible.

What I did

Removed the sender check.

How to test

We need to check out that all addons still work. I feel like this can break someones usage.

[storybookbot]

	Fails
🚫	PR is not labeled with one of: ["cleanup","breaking","feature","bug","documentation","maintenance","greenkeeper","other"]

Generated by 🚫 dangerJS

[usulpro]

@Gongreg I wonder why do you need to communicate between manager and (panel) addons via the channel? Why you can't use API for that?
I guess we use channel to organize communication with iframe (preview)
Maybe if existing API is not enough we need to add more?

[ndelangen]

I think this is perfectly understandable.

I'd like the channel to be the event-bus we can just throw events at, and have either side respond.

Sometimes an event is of interest to both the manager and preview.
Having to send an event on 2 different event-busses to get the actions to happen just doesn't make a lot of sense, right?

[codecov]

Codecov Report

Merging #1407 into release/3.3 will not change coverage.
The diff coverage is 100%.

@@             Coverage Diff              @@
##           release/3.3    #1407   +/-   ##
============================================
  Coverage        23.12%   23.12%           
============================================
  Files              253      253           
  Lines             5756     5756           
  Branches           690      690           
============================================
  Hits              1331     1331           
+ Misses            3928     3920    -8     
- Partials           497      505    +8

Impacted Files	Coverage Δ
lib/channels/src/index.js	100% <100%> (ø)	⬆️
addons/knobs/src/components/types/Object.js	5.81% <0%> (ø)	⬆️
...res__/update-addon-info/update-addon-info.input.js	0% <0%> (ø)	⬆️
app/react/src/client/preview/element_check.js	41.17% <0%> (ø)	⬆️
addons/knobs/src/components/types/Number.js	8.06% <0%> (ø)	⬆️
lib/ui/src/modules/ui/containers/layout.js	12.5% <0%> (ø)	⬆️
addons/info/src/components/markdown/htags.js	30% <0%> (ø)	⬆️
addons/knobs/src/components/types/Boolean.js	11.62% <0%> (ø)	⬆️
addons/info/src/components/Node.js	38.88% <0%> (ø)	⬆️
addons/events/src/components/Event.js	0% <0%> (ø)	⬆️
... and 16 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0e468a...3cc763b. Read the comment docs.

[usulpro]

is it a breaking change?

[Gongreg]

@usulpro, that I am not sure about. It can be breaking change for users
who were using same name events between manager <-> provider and provider <-> addons.

[ndelangen]

I doubt this is a breaking change actually

[usulpro]

I guess "writing-addons" guide encourages to use the one channel ID for addons.
At least Material-UI addon use the same ID for sending and for listening.
So if I understand right it will be broken.
Not sure about other addons... Do we know how many people use it the same way?

[Gongreg]

@usulpro, do you mean that they use same event name addon_name/eventName both for communication between manager addons and manager preview?

I mean for it to happen this way they would have to do two emits with same name in different locations.

[ndelangen]

Can you please fix the unit test that is failing?

[Gongreg]

@ndelangen, looks like it is fixed?

[shilman]

@Gongreg do you mind doing a test pass on the addons in cra-kitchen-sink?

[thani-sh]

Hi @Gongreg
We didn't have any special reasons to add the check. It was just part of the design at that time. If you guys think removing the check (event bus approach) will make things simpler, please go ahead. Just check the code where storybook sends/receives messages a bit.

[ndelangen]

I did a manual test pass when I approved this @shilman, but I guess example has changed quite a bit, so someone should retest.

Let's get this merged!

[ndelangen]

Tested, works fine!

@usulpro @tuchk4 If you need help getting your addon compatible, just ask for help on slack, ok?