Do Not Expose Schema through `changed` Event

[philippfromme]

What should we do?

We should not expose internal state through the changed event.

Why should we do it?

Users should not have access to internal state (cf. #234). Getting the schema should only be possible through saveSchema.

formEditor.on('changed', ({ schema }) => {
  // schema !== formEditor.saveSchema();
});

---

Related to #234

[nikku]

I kindly disagree, if I may.

If you see how changed is currently used it is not meant to be as a "saved" or something hook.

Users shall use the notification to trigger save of the form using proper means. If we document the payload of changed we shall remove that documentation. It is not supposed to be used `changed => dump contents anywhere.

[philippfromme]

I'm not sure if I get your point. As far as I understand changed is equivalent to elements.changed in bpmn-js. It's going to be used to export whenever the model has changed. It should not contain the schema itself as this suggests that an export using saveSchema isn't necessary.

[philippfromme]

If you see how changed is currently used it is not meant to be as a "saved" or something hook.

I mean, that's exactly what I was trying to say.

[nikku]

If your suggestion is to remove schema from the event payload for the sake of preventing it's miss-use, I agree. This could also mean, for example, that we mark it as internal.

If your suggestion is to sanitize the schema so it equals await formEditor.saveSchema() I argue we should not follow this path.

[nikku]

We have two options:

• expose internal state as an API or private field
• expose internal state via an event

Given how form-js (editor) we should pick the safest option (for our users).

[philippfromme]

My suggestion was to not expose the schema through any other means except saveSchema.

[philippfromme]

So I think we're on the same page.

[nikku]

All clear. Moving to backlog and marking as a potential spring cleaning topic.