Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More portable data payload #255

Open
chrisvxd opened this issue Nov 24, 2023 · 12 comments
Open

More portable data payload #255

chrisvxd opened this issue Nov 24, 2023 · 12 comments

Comments

@chrisvxd
Copy link
Member

chrisvxd commented Nov 24, 2023

Problem

The current data model is hard to reason about, which makes it less portable and harder to build custom renderers on top of. Some concerns:

  • zones are weird
  • The Root component is treated differently to the other components
  • DropZones rely on context or render props, which are React-only paradigms

Example of a custom renderers:

  • XML string (i.e. to produce React code)
  • Non-React outputs

Current data model

{
  "content": [
    {
      "type": "Columns",
      "props": {
        "id": "Columns-1234"
      }
    }
  ],
  "root": { "props": { "title": "Page" } },
  "zones": {
    "Columns-1234:left": [
      {
        "type": "Heading",
        "props": { "title": "Hello, world" }
      }
    ]
  }
}

Proposal

react-from-json implements a predictable AST based on, but not tied to, the React tree. It would allow for custom renders and increased portability.

Proposed data model

{
  "type": "Root",
  "props": {
    "title": "Page",
    "children": [
      {
        "type": "Columns",
        "props": {
          "left": [
            {
              "type": "Heading",
              "props": { "title": "Hello, world" }
            }
          ]
        }
      }
    ]
  }
}

Challenges

  1. Existing data payloads will be breaking
  2. Existing DropZone implementations will no longer work
  3. Any plugins modifying the Data payload will break
  4. If the user modifies the output in any way, this will break

Mitigations

  1. Implement a migration using Add mechanism for prop migration #41 (marked as a blocker) so Puck can still render old payloads
  2. Update the DropZone implementation with a new zone (or similarly named) field type.
  3. No mitigation - the plugin API is experimental
  4. TBD

Notes

  • Consider making the react-from-json model a portable standard
@chrisvxd chrisvxd pinned this issue Nov 24, 2023
@chrisvxd chrisvxd changed the title Consider recursive data payload More portable data payload Nov 24, 2023
@monospaced
Copy link
Member

This makes sense to me 👍🏻

But I do think we should have a plan for mitigating before commiting to this

  1. If the user modifies the output in any way, this will break

@BleedingDev
Copy link

I would check the output of BuilderIO. If it would be close to their, we could leverage their renderer components.

@chrisvxd chrisvxd unpinned this issue Dec 11, 2023
@jperasmus
Copy link
Contributor

The proposed nested structure is a lot easier to work with.

I suggest you consider nesting the data payload one level deep and storing the current version of puck in the root level. Any other additional metadata could be persisted this way as well which will be beneficial if you need to run data migrations for existing pages.

{
  "puckVersion": "0.14.0",
  "content": {
    "type": "Root",
    "props": {
      "title": "Page",
      "children": [
        {
          "type": "Columns",
          "props": {
            "left": [
              {
                "type": "Heading",
                "props": { "title": "Hello, world" }
              }
            ]
          }
        }
      ]
    }
  }
}

@Tuyentran2k1213
Copy link

Hi @chrisvxd is this on working ??? this feature will be needed

@chrisvxd
Copy link
Member Author

chrisvxd commented Feb 2, 2024

@Tuyentran2k1213 it's on our radar but we haven't started efforts yet! Hoping to look at it over the next few months.

@simon-virkesborsen
Copy link

@chrisvxd Would a good first step to be implementing this with a feature flag to avoid the breaking change at first?

@chrisvxd
Copy link
Member Author

@simon-virkesborsen perhaps, but it's so core to the Puck implementation that it might cause a lot of overhead to retain two separate implementations.

One thing I'm considering is retaining the internal behaviour and just mapping the external data payload. That would enable the use of a feature flag.

@mkilpatrick
Copy link
Contributor

In addition to the new format (love the puckVersion idea), could there be a non-puck field included like metadata to allow users to add additional info that we may require for our own services? Puck would ignore this field but allow additional custom attributes external systems could use.

This could be extended to the component config too.

As a concrete example, I'm looking to eventually add role-based controls where certain users can only control certain components/fields. I'm not sure if something like this is already on your roadmap, but with some custom metadata we could at least roll our own thing in the meantime. I'm sure there are countless other use cases for this too.

@shannonhochkins
Copy link

In addition to the new format (love the puckVersion idea), could there be a non-puck field included like metadata to allow users to add additional info that we may require for our own services? Puck would ignore this field but allow additional custom attributes external systems could use.

This could be extended to the component config too.

As a concrete example, I'm looking to eventually add role-based controls where certain users can only control certain components/fields. I'm not sure if something like this is already on your roadmap, but with some custom metadata we could at least roll our own thing in the meantime. I'm sure there are countless other use cases for this too.

I think this is a good start, there might also be internal config you might want to update too, like the dropzone properties or if a component should be draggable within a dropzone etc - might need to think about where this lives under this new proposed structure

@janbaykara
Copy link

Hi all. Firstly, big respect to the Puck project so far. This is all very cool.

We're exploring using Puck as a React-embedded editor on top of Wagtail, the Django-based CMS. Wagtail stores page content as Revisions, a JSON blob that serialises the Django model. It additionally has a concept of StreamField for block-based content storage.

From a purely self-motivated perspective, I'd wonder if there can be some convergence between these data structures and Puck's.

From the open source ecology perspective, @chrisvxd's idea of mapping Puck data structures to other ones strikes me as a versatile idea. Mapping packages, between Puck and third party data formats (e.g. puck-to-wagtail and wagtail-to-puck), would be a very cool addition to the ecosystem.

@mkilpatrick
Copy link
Contributor

Wondering if this is currently in the works or when this might be completed. Once the format changes it will be hard to move existing data to the new format unless there's a migration plan. Either way, it'd be easier to wait for this before using Puck in a Production setting.

@chrisvxd
Copy link
Member Author

@mkilpatrick we'll ensure there is a robust migration plan. Not yet in the works, but I'm hoping to begin working on it relatively soon (can't commit to timeline yet, as many client and personal commitments).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants