Skip to content

[Content management] client POC#18

Closed
Dosant wants to merge 5 commits intosebelga:content-management/core-server-pocfrom
Dosant:d/2023-01/content-management/client-poc
Closed

[Content management] client POC#18
Dosant wants to merge 5 commits intosebelga:content-management/core-server-pocfrom
Dosant:d/2023-01/content-management/client-poc

Conversation

@Dosant
Copy link
Copy Markdown

@Dosant Dosant commented Jan 16, 2023

Summary

built on top of wip server POC but with client cache layer

The tech doc for the client-side content service: https://docs.google.com/document/d/13aOspEx_yGcdaRwXZTAOCaiPuAIZlcrkZopvR0cr2_k/edit#

The client POC consists of two independent parts:

  • Content Type Registry – A client-side registry for registering a content type. Registrars provide presentational information about the content type such as display name, description, icon, etc.
  • Content Client – A client-side service and set of helpers exposed to other plugins and our components for querying and mutating the content. Includes features such as cache, requests deduping, retries, etc.. The tech doc and the POC proposes to use react-query internally and for shaping an API

Untitled-2023-01-05-1448-3

import type { ContentTypeDetails } from './types';
import { ContentType } from './content_type';

export class ContentRegistry {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I wonder why we need the registry client side. Couldn't we have this on the server and a method (cached) to fetch once those details from the server?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good question. I'll put this question here https://docs.google.com/document/d/13aOspEx_yGcdaRwXZTAOCaiPuAIZlcrkZopvR0cr2_k/edit# and will iterate on it

import type { SearchIn } from '../../common';
import { CreateIn, CreateOut } from '../../common';

const contentQueryKeyBuilder = {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: Can we maybe shorten this to queryKeyBuilder?

return this.queryClient.fetchQuery(this.contentQueryOptionBuilder.get<ContentItem>(type, id));
}

get$<ContentItem extends object = object>({ type, id }: { type: string; id: string }) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I am not sure I fully understand what those observable version of the method do 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Have you seen https://docs.google.com/document/d/13aOspEx_yGcdaRwXZTAOCaiPuAIZlcrkZopvR0cr2_k/edit#heading=h.oijoa468kc4q ?

The idea is that cache is reactive, so the main way to access data would be through the react hook or RXJS observable. Regular promise based method can be used in the cases where reactivity is not needed

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ah I see, indeed this is outside React context so we need a way to make it reactive. 👍


return new Observable<QueryObserverResult<TData, TError>>((subscriber) => {
const unsubscribe = queryObserver.subscribe(
// notifyManager is a singleton that batches updates across react query
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Strange I can't find anything about notifyManager in their docs... 🤔

Screenshot 2023-01-30 at 13 52 49

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't think it is a public api. It is exported from query-core and is used by react (or other libs) implementations. I learned about it from the useQuery hook source

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ok. Isn't it risky to not use a public api?

}

export class ContentManagementPlugin implements Plugin {
private rpcClient: RpcClient | undefined;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We probably don't need this private property anymore

@TinaHeiligers
Copy link
Copy Markdown

cc @TinaHeiligers @jloleysens

@Dosant Dosant closed this Feb 7, 2023
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.

3 participants