fix(Canvas): in/out event names were swapped

[jiayihu]

Event names were swapped during the migration.

EDIT:

Link to the source of the bug:
b6f076c#diff-c24dadcf5292c5b8555816413000f03a691de5a3caf99544bd9d33eed564a30eR1320-R1345
compared to
b6f076c#diff-72332330d8e00b2fda6e57d000a93e4e8dd8a46fdc8bcbdb3320c23b1509f8b5L1199-L1228

[ShaMan123]

fixed types and added tests

[jiayihu]

Why was the type fix reverted? If you use the optional operator, it won't complain if the nextTarget/previousTarget is not passed at all, even if undefined. That's why it was not giving an error. Whereas nextTarget: fabric.Object | undefined means that it must be explicitly provided, even if undefined, which is what we do.

[jiayihu]

@ShaMan123 This could have been targetSpy.mock.calls.map(([event]) => event), then even use toMatchInlineSnapshot for a much smaller and nicer snapshot, while being convenient.

[asturur]

yes.
stop toMatchSnapthot.

[ShaMan123]

Why was the type fix reverted? If you use the optional operator, it won't complain if the nextTarget/previousTarget is not passed at all, even if undefined. That's why it was not giving an error. Whereas nextTarget: fabric.Object | undefined means that it must be explicitly provided, even if undefined, which is what we do.

I didn't look into it but build was failing.
My bad

[asturur]

@ShaMan123 @jiayihu you are welcome to merge laser focused fixes like this but:

• The source of the commit that broke the code shouldn't be hidden in comments, needs to be in PR description
• as pointed out in comments snapshot tests are uneffective so please let's stop adding snapshot tests for those kinds of thing.

We can simply say:
theMock has been called with 'mousein' or whatever, a blanket cover all of the event data isn't a good idea imho