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

Proposal for GraphQL API for cross-platform rich content #26

Merged
merged 4 commits into from
Feb 25, 2019

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented Sep 19, 2018

⚠️ Addendum 2019/2/25: Added description of MVP Phase 1 implementation for upcoming PWA Studio releases.

This is a proposal for supporting CMS in our GraphQL layer through a common interface type designed to eventually interoperate with PWA Studio, Adobe Experience Manager, and/or other systems where business users create rich content with a WYSIWYG. It proposes a GraphQL format for traversing a tree and learning metadata about node implementations. A proposed schema:

interface VisualContentNode {
  role: String!
  depth: Int
  preload: [String]
  descendentRoles: [String]
  children: [VisualContentNode]
  html: String
}

An example request-response:

Request:

fragment NodeMetadata on VisualContentNode {
  nodeId
  role
  depth
  preload
  descendentRoles
}

content(nodeId: 1) {
  ...NodeMetadata
  html
  children {
    ...NodeMetadata
  }
}

Response:

{
  "content": {
    "nodeId": 1,
    "role": "Column",
    "depth": 4,
    "preload": [
      "banner1.jpg",
      "diagram.svg",
      "thumbnail001.jpg",
      "thumbnail002.jpg",
      "video.mp4"
    ],
    "descendentRoles": ["Banner", "Accordion", "Video", "Image", "Row", "Text"],
    "children": [
      {
        "role": "Banner",
        "depth": 0,
        "preload": ["banner1.jpg"],
        "descendentRoles": []
      },
      {
        "role": "Accordion",
        "depth": 3,
        "preload": [
          "diagram.svg",
          "thumbnail001.jpg",
          "thumbnail002.jpg",
          "video.mp4"
        ],
        "descendentRoles": ["Video", "Image", "Row"]
      },
      {
        "role": "Text",
        "depth": 0,
        "preload": [],
        "descendentRoles": []
      }
    ],
    "html": "Hello world! Text has already loaded."
  }
}

@zetlen
Copy link
Contributor Author

zetlen commented Sep 19, 2018

@kandy suggests that instead of recursing through a tree, the API represent the nodes as an array with reference pointers to child nodes, and the client reconstruct the tree as necessary. Then depth could be controlled with a filter on the depth property:

query getContentNodes($id: String!) {
  nodes(filter: { depth: { lt: 4 }, and: { rootNodeId: { eq: $id }}) {
     depth
     role
     childNodeIds
  }
}


## Proposal

Add an interface to the GraphQL schema representing a recursive tree of PageBuilder content nodes. Each PageBuilder block type should add a concrete type to the GraphQL schema which implements this interface and adds additional fields for any required configuration. PWAs initially query only against the interface, and can therefore only receive the metadata. In the first few content loads, the PWA must make a "followup" request with more concrete query depth. As its cache grows, it does fewer and fewer of those requests.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, we avoid using fragments syntax for querying relevant fields depending on the actual implementation of the generic content node interface. Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the list of potential implementations is unknown by the client. Extensions that add new PageBuilder blocks could add new implementations. Those implementations would have to include a GraphQL fragment that a PWA Studio PWA could use to add to its CMS query. That can happen at build time, but it should also be possible to do so at runtime.


* **Why not use GraphQL introspection for this?**

- Per the GraphQL specification, the introspection query must provide only metadata, and not any data from resolvers. Implementing "partial introspection" via type tag names is a more efficient way of resolving recursive content nodes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introspection will also be disabled in production.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should bring this up in a different issue, but we should at least permit the "standard introspection query" in production, which GraphQL clients use to generate documentation. It should be enough to cache the results of this query, since it will be invariant in production.

@melnikovi
Copy link
Member

melnikovi commented Nov 11, 2018

@zetlen please describe rendering current master format and returning definitions of React components based on offline discussion.

@zetlen
Copy link
Contributor Author

zetlen commented Jan 9, 2019

@melnikovi Done!

Copy link

@jednano jednano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions.

}
```

3. Re-render the fully hydrated second level of component when that query returns results.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to avoid a re-render? I'm concerned about the page jumping as new content loads. I know you're not a fan of SSR, but it sounds like it would solve this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to avoid this, the HTML fallbacks would have to have dimension. I think that the RichContent component could load a root PageBuilder stylesheet which would apply default dimensions.

As more PageBuilder components become "known" to the PWA runtime, it could add fragments to VisualContentNode queries like ...on PageBuilderGallery {} etc. which would get more and more specific configuration, including dimension, on the first render...

```


3. Re-render the fully hydrated Slider component when that query returns results.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here. Always concerned about re-renders.


3. In series:

4. Dynamically load and inject the `PageBuilder_Slider` React Component. (The bundle system and service worker will load from their caches if possible.)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much content are we talking about here? I'm concerned about performance issues of putting too much responsibility on React to render things on the FE instead of on the first page load. I only speak from experience, because I've been 🔥 before on this point.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Slider, we're talking about 3-4 Slides usually, with something like 10-20 is going to be a very rare edge case.

But the thing is is that the Slide itself can contain more internal components, like buttons, headings, texts, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd have to see how it performed. The HTML fallback would already be in the DOM node that the Slider component takes over, so I would hope the React reconciler would reuse as much of it as possible.

How much content are we talking? The point is, we don't know at runtime.

4. For initial render, `RichContent` injects the sanitized fragment.
5. After initial render, `RichContent` visits the DOM in the fragment to build a tree of PageBuilder rows, columns, widgets, and configurations.
6. When the content tree is finished, `RichContent` dynamically imports the React components corresponding to all the PageBuilder widget types present in the content.
7. When the runtime has loaded the components, `RichContent` re-renders the content tree as instances of live React components instead of HTML.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.


3. Re-render the fully hydrated second level of component when that query returns results.

Subsequent requests for any content which lists any of those components in its `descendentRoles` will automatically include those fragment and clauses in the `content` query. Therefore, subsequent requests will contain the necessary configurations on the first pass.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to measure performance on the prototype. I feel like network communication expenses would be higher than potential slowness of getting large JSON in one request. I think we wouldn't have a lot of content. This also makes caching less efficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely measure the performance, and change approaches if it's terrible.

I think that caching might be better with GraphQL. If VisualContentNodes have unique IDs, then Apollo cache implementations can store their representations incrementally. Then, the PWA would reuse references to the same content entity.


### Considerations

- The workaround involves DOM parsing and large string comparisons for update filters, which are both expensive operations which PWAs try to avoid.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add pros and cons for each of the solutions?

@melnikovi
Copy link
Member

@zetlen could you provide more context on "common interface type designed to eventually interoperate with PWA Studio, Adobe Experience Manager, and/or other systems where business users create rich content"? How Adobe Experience Manager and other systems will use proposed API?

@melnikovi melnikovi self-requested a review February 14, 2019 16:07
@melnikovi melnikovi merged commit caae627 into magento:master Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants