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

[Files] Refactor how types are shared between server and client #141580

Merged

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Sep 23, 2022

The current implementation declares all types under common and shares these with public and server. This results in some duplication because types can be derived from the @kbn/config-schema objects, so we can use them as the source of truth of endpoint input values.

Refactor out some repetitive declarations: convert the way we declare the share types to derive their shape from the runtime type checks that live on the server.

The convention is, per endpoint:

  1. Declare runtime type checkers for inputs (body, query, params)
  2. Feed the runtime values into the CreateEndpoint type as a generic argument
  3. Use the result of CreateEndpoint, i.e., Endpoint, to derive types for the client

@jloleysens jloleysens added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed Team:AppServicesUx v8.5.0 feature:Files labels Sep 23, 2022
* main:
  [Files] Use self destruct flag in upload component (elastic#141366)
  [Synthetics] Unskip certs flaky test (elastic#141423)
  updated openapi for bulk action responses (elastic#141566)
  Fix page responsiveness for smaller screen sizes (elastic#141471)
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@jloleysens jloleysens marked this pull request as ready for review September 23, 2022 11:48
@jloleysens jloleysens requested a review from a team as a code owner September 23, 2022 11:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

Copy link
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

@jloleysens jloleysens merged commit 6429468 into elastic:main Sep 23, 2022
@jloleysens jloleysens deleted the files-restructure-endpoint-types-ii branch September 23, 2022 14:13
kibanamachine pushed a commit that referenced this pull request Sep 23, 2022
)

* migrated uploads component and added new way of typing and creating an endpoint

* updated create endpoint types

* convert delete endpoint

* convert download endpoint

* convert get by id endpoint

* convert list endpoint

* convert update endpoint

* convert public facing download

* convert share get

* convert list shares

* missed a spot

* remove unused import

* convert share endpoint

* convert unshare endpoint

* convert find find endpoint, fix types and fix use of empty value check

* convert metrics

(cherry picked from commit 6429468)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Sep 23, 2022
) (#141643)

* migrated uploads component and added new way of typing and creating an endpoint

* updated create endpoint types

* convert delete endpoint

* convert download endpoint

* convert get by id endpoint

* convert list endpoint

* convert update endpoint

* convert public facing download

* convert share get

* convert list shares

* missed a spot

* remove unused import

* convert share endpoint

* convert unshare endpoint

* convert find find endpoint, fix types and fix use of empty value check

* convert metrics

(cherry picked from commit 6429468)

Co-authored-by: Jean-Louis Leysens <jeanlouis.leysens@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed feature:Files release_note:skip Skip the PR/issue when compiling release notes v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants