Mavenomics / MavenWorks

Agile Dashboarding, anywhere
https://mavenworks.com
GNU General Public License v3.0
18 stars 2 forks source link

Changing the type of some globals may completely break the globals editor #78

Open quigleyj-mavenomics opened 5 years ago

quigleyj-mavenomics commented 5 years ago

Changing the type annotation of a non-serializable value (such as Table or Row, which cannot be directly passed to JSON.stringify, or Color, which cannot round-trip), from their actual type to Any or Object will break the editor in weird ways.

For Table and Row, the editor will completely break. An unhandled serialization error is thrown during the rendering of FallbackTypeEditor, which bubbles up to the React root and destroys the entire component. As long as that type annotation remains "Any", the Globals Editor can never be re-summoned for exactly this reason. Worse still, dashboard can be saved with this bad state, requiring manual editing to fix.

For Color, the editor will still work, and even show a semi-readable value. However, this value does not round trip! It is merely coincidental that the Color class will not cause JSON.stringify to throw any errors, and does not reflect any intended functionality in the class. If no edits are made, the change can be reverted by switching back to the Color type and retaining the original value. However, if any changes are made, the old value will be lost.

This was previously tracked on the old, non-public JIRA as JOVIAN-507:

Repro:

  • Create a new global, type it as “Table”
  • Bind it to a table editor
  • Edit the table
  • Open the globals editor, and try changing the type of the global to “Any”

Expected behavior:

  • The global value is either cleared or wrapped in a {“typeName”: “Table”, “value”: …} construct.

Actual behavior:

  • A serialization error bubbles up to React, killing the component. Applying the change will poison the globals, breaking the dashboard.

A mitigation was applied to hide the Any type once another type has been selected, but that mitigation merely hid the issue.