fix(lib): Better error message when message can not be serializable

[demetriusfeijoo]

What?

Improve exception's message when an object is not cloneable (accepted by postMessage).

Why?

JIRA: EXT-1603

Some object structures are not allowed to be sent thru windows.postMessage() function, raising a DataCloneError exception.

However, the raised exception is not so clear to understand since this error is too generic and doesn't give so much useful information to the developer.

Exemple of a wrong message type:

Output Before:

Output After:

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
plugin-sandbox	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 25, 2023 5:29pm

[eunjae-lee]

This PR is not refactoring the code, but it's more of an improvement on how we handle the error situation. So I renamed the title to "fix".

[demetriusfeijoo]

You're right @eunjae-lee .

Thanks for changing it 🎯.

[johannes-lindgren]

I don't know if tests for this function make sense. It contains side-effects, so it will be difficult to test, and tests will tend to test internal implementation. I'd recommend extracting postToContainer outside createFieldPlugin. When you do, pass window.postMessage as an argument, so that you don't need to mock anything. That will be much more testable.

I think we can remove the test in this file. They have some issues, for example:

expect(cleanupFieldPlugin()).toEqual(undefined)

Is not needed because the return value is already checked by TypeCcript. Also, it could start to fail if the cleanup function returned a value, which would be okay with Typescript, as the return signature is void: this is valid:

const fun: () => void = () => 123

expect(onUpdateStateMock).toHaveBeenCalledWith({
      type: 'error',
      error: new Error(
        `The window is not embedded within another window. Did you mean to open the field plugin in the sandbox? ${sandboxUrl()}`,
      ),
    })

This would fail if the message changed slightly, but that is ok.

expect(onUpdateStateMock).toBeCalled()

seems to test something else than what described in the it() function call.

[johannes-lindgren]

Good!

[demetriusfeijoo]

Thanks for the detailed explanation @johannes-lindgren. 🙌

I'm going to take a look at these points.

[demetriusfeijoo]

It came from the main branch