Skip to content

[CM] Setup client side content client#150171

Merged
Dosant merged 11 commits intoelastic:mainfrom
Dosant:d/2023-02-02-cm-setup-client
Feb 7, 2023
Merged

[CM] Setup client side content client#150171
Dosant merged 11 commits intoelastic:mainfrom
Dosant:d/2023-02-02-cm-setup-client

Conversation

@Dosant
Copy link
Copy Markdown
Contributor

@Dosant Dosant commented Feb 2, 2023

Summary

Partially addresses #149216, #149939
Closes #149217

Setup basic client-side content client (client, cache, react content and hooks) based on the latest API from @sebelga's server-side and rpc POC. We assume that the POC isn't far from the initial version that we are going to merge to main

The client side content client is based on the proposal described in private doc

Initial content client implementation scope:

  • We start from using tanstack/query for client side caching, content queuing and mutating . Also use it as a basis for the API. We start from their defaults and will adjust defaults as we go. Basically our content client would be a helpful wrapper around tanstack/query (at least initially)
  • We start from exposing minimal query lib API surface, but we will gradually expose more as needed
  • For content retrieval we support promise based, RXJS observable based and react hook based methods
  • For the RPC/api I copied over the latest from server-side and rpc POC assuming it won't change much.
  • no optimistic update or automatic invalidation after updates yet (this will be later)
  • just get and create for now

@@ -0,0 +1,72 @@
/*
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This file is copy paste from the server-side POC #148791.

Assuming this won't change much when initial server-side is merged to main. Also won't be a problem if this is changed a lot

import { API_ENDPOINT } from '../../common';
import type { GetIn, CreateIn, ProcedureName } from '../../common';

export class RpcClient {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This file is copy paste from the server-side POC #148791.

Assuming this won't change much when initial server-side is merged to main. Also won't be a problem if this is changed a lot

@Dosant Dosant added Feature:Content Management User generated content (saved objects) management release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// labels Feb 2, 2023
@Dosant Dosant requested review from sebelga and vadimkibana February 2, 2023 13:58
@Dosant Dosant marked this pull request as ready for review February 2, 2023 13:58
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Feb 6, 2023

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

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

LGTM

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Feb 7, 2023

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

Great work @Dosant ! I left a few nit comments but looks good 👍

it('calls rpcClient.get with input and returns output', async () => {
const input: GetIn = { id: 'test', contentType: 'testType' };
const output = { test: 'test' };
rpcClient.get.mockImplementation(() => Promise.resolve(output));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: could be rpcClient.get.mockResolvedValueOnce(output);

import { createRpcClientMock } from '../rpc_client/rpc_client.mock';
import { ContentClient } from './content_client';
import type { GetIn, CreateIn } from '../../common';
import { lastValueFrom } from 'rxjs';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we put absolute imports above relative ones?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Was it supposed to be linted? 🤔

);

describe('useCreateContentMutation', () => {
test('should call rpcClient.get with input and resolve with output', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: ... "rpcClient.create"

public start() {
return {};
public start(core: CoreStart, deps: StartDependencies) {
// don't actually expose the client until it is used to avoid increasing bundle size
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Feb 7, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
contentManagement 5 22 +17

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
contentManagement 0 1 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
contentManagement 1.2KB 1.2KB +12.0B
Unknown metric groups

API count

id before after diff
contentManagement 5 22 +17

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Dosant Dosant merged commit 6d96f51 into elastic:main Feb 7, 2023
@Dosant Dosant mentioned this pull request Feb 7, 2023
3 tasks
@kibanamachine kibanamachine added v8.7.0 backport:skip This PR does not require backporting labels Feb 7, 2023
@Dosant Dosant mentioned this pull request Feb 7, 2023
4 tasks
Dosant added a commit that referenced this pull request Feb 15, 2023
…hods (#151041)

Nothing to write home about - just a bit more boilerplate setup code.
Method signatures and RPC related interfaces are expected to be changed
later with an initial RPC layer implementation.

- Follow-up to #150171, adding a
dummy setup for `update` `delete` and `search` methods. **The `IN` and
`OUT` args will be changes when we glue these to the rpc service and its
types**
- Also clean up unit tests a bit to have setup logic contained in a
helper instead of scattered in global scope
justinkambic pushed a commit to justinkambic/kibana that referenced this pull request Feb 23, 2023
…hods (elastic#151041)

Nothing to write home about - just a bit more boilerplate setup code.
Method signatures and RPC related interfaces are expected to be changed
later with an initial RPC layer implementation.

- Follow-up to elastic#150171, adding a
dummy setup for `update` `delete` and `search` methods. **The `IN` and
`OUT` args will be changes when we glue these to the rpc service and its
types**
- Also clean up unit tests a bit to have setup logic contained in a
helper instead of scattered in global scope
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Content Management User generated content (saved objects) management release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CM] Setup browser-side React integration

6 participants