feat: Add Dynamic Pages

[neatbyte-vnobis]

Changes

Implements issue "Dynamic Pages".

How Has This Been Tested?

Manual

Documentation

None

[Pavel910]

• would it be possible to move the template model data loading into the DynamicSourceContentPlugin (currently you’re loading the template model with the editor), in this if statement: packages/app-page-builder/src/templateEditor/Editor.tsx:125. It would be better if this functionality was co-located with the rest of the feature logic, so it’s easier to turn off, or replace in the future.
• create a dedicated atom or state provider for the sourceModel data, don’t store it on the page template atom directly
• changing the order of elements triggers a new GraphQL query, because the query is a plain string, and you do a simple !== comparison before doing a RunQuery query. Change of field order should not run a query.
• let’s create a better GQL query on the API side, for example, getDynamicPageData, dedicated to loading data by modelId, and an array of field paths like: [“title”, “author.firstName”, “author.lastName”]. It will reduce the amount of client side processing, and HTTP requests to the API. You will be able to build the query and immediately execute it, without sending new HTTP requests. It will also help debug queries through GraphQL Network plugin, because you’ll have proper variables here, instead of a concatenated string (see screenshot). Lastly, we’ll be able to test those utilities. Right now they’re not testable (getNestingByPath is a function which can potentially run dozens of queries to the API; we have clients with massive content models (hundreds of fields, dozens of ref fields, etc.). if you move query and data resolution to the API:
  • it can be tested as part of our test suite and
  • a lot of chained calls to resolve content models and ref fields can be executed in one http request
  • you won't need the readApolloClient addition in the CMS context at all, because page editor will be talking to the Page Builder GraphQL, not CMS. Same goes for the website app. With a custom query to resolve block data, we won't need to add CMS clients to the website app.
    
• change this to preview endpoint, so we can also use draft content for preview: packages/api-page-builder/src/plugins/cmsQueryingPlugin.ts:85
  • for data loading from the client side, we’ll need to have 2 queries: one for “editor” and one for the “website” app. The “editor” query should return the preview endpoint data, meaning we can preview draft content, and “website” query must work with the read CMS endpoint (published content)
• Let’s rename templatePageData to dynamicSource, where modelId is a required field (when we create a template, we already know the model, so modelId will always be present):
  • dynamicSource?: { modelId: string, entryId?: string }
  • if dynamicSource is set, it MUST have a modelId.
• Seems like PageTemplate.modelId is not used. If so, remove it from the type: packages/api-page-builder/src/types.ts:898 and here: packages/app-page-builder/src/templateEditor/state/templateAtom.ts:7
• unreadable code. Extract dynamicSource once, and just use that everywhere. There are many examples of this throughout the code, so please do clean this up. Also, with our current babel setup, this transpiles into a massive amount of code, which makes the bundle a lot larger. So let's be careful with these optional chaining operators and not overuse them.

• use the existing useElementWithChildrenById hook: packages/app-page-builder/src/editor/plugins/elements/carousel/PeCarousel.tsx:56
• remove the /index from imports "@webiny/app-headless-cms/index"
• create a useModelById(modelId) hook:
  • packages/app-dynamic-pages/src/utils/composeDynamicApi.ts:6
  • packages/app-dynamic-pages/src/utils/getNestingByPath.ts:29
  • as a rule of thumb, if you see you need to reach for Recoil atoms, just go ahead and create hooks to interact with that state.
• add descriptions to your functions/hooks, which explain the purpose of the function. For example: composeDynamicApi - what does it do? what is templateWhereField and where does it come from? Took me a decent amount of time to figure out when this parameter is set, and I had to add console logs and randomly click through the UI to finally stumble upon that.
  
• we need to find a better solution for this, because this doesn't work at all for me, as title doesn't exist in the GraphQL schema, and it's not, in general, a good approach:
  

[Pavel910]

Left a few comments, but overall it's good progress 👍

[Pavel910]

This file is identical for both pageEditor and templateEditor, it's an exact copy. Can we extract it into a shared location? Maybe even app-dynamic-pages package, since the logic here is entirely focused on dynamic pages. I was hoping we can have app-dynamic-pages package be entirely responsible for everything related to dynamic pages, and just hook into the editors via plugins.

[adrians5j]

I'm not assigned as a reviewer on this project, but still I quickly peeked.

Commented on a couple of things.

Thanks!

[adrians5j]

A bit concerned with this line here because this new package introduces a whole bucket of new dependencies, which are potentially gonna get bundled and bloat the public website code. Which of course then affects the SEO and all that related stuff.

I checked what's the usage of the package, and noticed we're using it because we need useDynamicData hook.

So, what I'm thinking are two things.

We extract the React hook and related code into a separate package, like app-dynamic-pages-hooks. That package would not include any UI nor all of the big and irrelevant deps in its package.json, like @emotion/styled, @fortawesome/react-fontawesome, or the huge @webiny/ui.

The app-page-builder-elements package would then include that, and would no longer include app-dynamic-pages.

We could take this even further and not have the new package in app-page-builder-elements at all. We can simply adapt the same approach that's used for example here: packages/app-page-builder/src/render/plugins/elements/button/index.tsx.

Basically, instead of hardcoding deps into page elements, we allow them do be passed on the element construction. For example, with the linkComponent prop, we allow users to pass a custom Link component. This will never be needed if users are using Webiny and its website rendering capabilities, but if a user is using, say, Next.js to render a page, then here they would most probably want to pass Next.js's Link component. This approach will also prevent things like Apollo React client being installed in external projects, and allow users to implement their own way of fetching data, if needed.

I hope I'm making sense. I know some of these are NTH, and I'd say that at this moment, let's try to maybe implement at least the first part. That way we're making at least some effort in preventing the website bundle to get even larger that it already is. At the moment, there's really a lot of extra stuff that degrades the overall performance and SEO score of the public websIte, we'll need to address this sometime in the future. I can see it happening when we launch dynamic pages, and people start using more PB to render their website.

We can consider the 2nd part as a NTH.

Let me know if a call is needed potentially.

UPDATE:
Another thought just crossed my mind. By just implementing the 2nd step I mentioned in the above text, we'd be able to get rid of the app-dynamic-pages dep from app-page-builder-elements, and we'd also probably be fine when it comes to the code that gets bundled ultimately on the public website. I checked what is the code that gets imported by useDynamicData hook, and it's basically our code plus Apollo client, which is fine to get included on the website side (it's a must). I'd maybe even consider this as the way forward at this moment. Note that the useDynamicData hook can also be provided via the Page Elements provider component, and doesn't necessarily have to be provided for each of the page element components separately.

[adrians5j]

Hm why this change? 🤔