cortex-cms / cortex

:pencil: A headless, multitenant dynamic content platform powered by Rails, GraphQL and Elasticsearch
https://docs.cortexcms.org/
Apache License 2.0
32 stars 6 forks source link

Proposal: Flatten/Standardize ContentType Decorator's Object Shape #539

Open MKwenhua opened 7 years ago

MKwenhua commented 7 years ago

Currently there are a few issues with how Decorator data is currently structured that make it more difficult to both render and customize.

Some of these issues are as follows:

  1. Currently there is an implicit dependency on how decorator data is structured to each FieldType's cell templates, some of which contain deep object nesting which throw template errors when not present. For example:
                  {
                    "id": blog.fields.find_by_name('Body').id,
                    "render_method": "wysiwyg",
                    "input": {
                      "display": {
                        "styles": {
                          "height": "500px"
                        }
                      }
                    }
                  }

One of the issues with data other than id and render_method is there is no standardization across different FieldTypes for attribute data. Additionally the deep nesting can cause null errors as data tried to get accessed data[:input][:display][:styles][:height] could easily break if any of those expected Objects are not present.

Proposed Change:

Given that these elements will soon be rendered in React we can take advantage of a few ES7 features that allow encapsulation while still remaining flat. For example we could set the metadata to be shaped something like this:

id: "8a810-72726hnva-jhvjssdcs"
render_method: "wysiwyg"
style: 
   height: "500px"
id: "blog_body_editor"
data-gtm: "7828734687346834"

This data could then be passed into a Field component that can use the rest operator to bunch all data that is not id or render_method into an object called attrs.

    const {id, render_method = 'default', ....attrs} = this.props; 
    // A look up object for different TextFieldType renders
   const FieldComponent = TextFieldTypeRenders[render_method]

    return <FieldComponent  fieldData={ FieldsLookUp[id] }  {....attrs}/>
  1. There is inconsistencies between the Index and Wizard Decorators in how their data is laid out. For example:
[
            {
              "name": "Author",
              "grid_width": 2,
              "cells": [{
                          "field": {
                            "method": "creator.email"
                          },
                          "display": {
                            "classes": [
                              "circular"
                            ]
                          }
                        }]
            },

Additionally having cells be an array while easier for ruby makes look up and access more complex and inefficient for tracking and updating changes to the data in React.

Proposed Change:

We should make data as flat as possible and keep all data on the first level and achieve nesting via reference keys.

MKwenhua commented 6 years ago

Final Comments

So it seems that I have sort of covered why deeply nested objects can throw but below I would like to explain each of the main reasons why this can cause problems as well as a potential remedy in case nesting is absolutely necessary.

Why Nesting Can Lead To Issues:

Unknown Traversals And Implicit Dependencies

So lets say that each fields metadata is randomly created with different nested values. What this means is that each field cell/component when being rendered needs to consider what the object's shape is. Because of these different nested states a implicit dependency is created between how the data is stored in the database and how the data is accessed within both Rails Cells or React.

An issue that arises when data is nested is that any change or removal of a nested key runs a risk of accessing a property on a null or undefined property. This issue can be avoided however when using flat 1D objects because accessing a nonexistent key on an object will not throw an exception. Additionally with ESNext completely flat objects can be treated as separate objects using the rest operator:

const metadata = { 
  render_method: "wysiwyg",
  field_id: "8a810-72726hnva-jhvjssdcs",
  style:  {borderRadius: "8px"}, 
  src: 'an_image_url',
  id: "blog_body_editor",
  data-gtm: "7828734687346834"
}

//now just pull out the non attribute keys as such:
const { render_method, field_id, ...attrs } = metadata;

Propensity Towards Data Loss

In Redux every dispatch generates and creates a new object created from state. Often times when working with nested objects, devs accidentally replace the object with just the changes. For example:

const TheState = {
    connected: true,
    user_profile: {
       friends: 6,
       name: 'Richard Feynman',
       sandwich_type:  'Pastrami'
    }
}

//Now within the reducer for action NEW_FRIEND_ADDED

{
. . .
case NEW_FRIEND_ADDED: {
    return {
       ...state,
       user_profile: {
          friends: state.user_profile.friends + 1
       }
    }
}
. . .
}

This error would result in the new state being:

const TheState = {
    connected: true,
    user_profile: {
       friends: 7
    }
}

But this can be fixed by doing a spread of the rest of the object:

{
. . .
case NEW_FRIEND_ADDED: {
    return {
       ...state,
       user_profile: {
          ...state.user_profile,
          friends: state.user_profile.friends + 1
       }
    }
}
. . .
}

So as you can see the deeper you nest the more unwieldily it gets.

Prevents Good Abstractions

If data is flattened abstractions below can be built to make rendering much easier. For example lets assume we have an array of field types:

   Step.FormFields.map((field, i) => {
      const {render_method, field_id, ...data} = field.metadata;
     return FieldTypeRendMethods[field.type][render_method](data)
    })
MKwenhua commented 6 years ago

@toastercup I will add some more comments when I get home tonight, so just wait till tomorrow morning before taking me off the contributor list. I need to pack up my things

toastercup commented 6 years ago

@MKwenhua take your time - I don't have to take you off contrib right away