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

Create server-side docLinks service #123818

Merged
merged 19 commits into from
Feb 1, 2022

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Jan 26, 2022

Summary

Fix #95389

✔️ Depends on elastic/docs#2360

  • Introduce a new @kbn/doc-links package
  • Move the doc links types and definition from src/core/public/doc_links to it
  • Create a server-side DocLinks service

Note: due to the amount of docLinks changes, I will resolve all the conflict at the same time before merging once the PR gets approved

Checklist

@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.1.0 release_note:skip Skip the PR/issue when compiling release notes labels Jan 27, 2022
@pgayvallet pgayvallet marked this pull request as ready for review January 27, 2022 20:17
@pgayvallet pgayvallet requested a review from a team as a code owner January 27, 2022 20:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

Comment on lines +27 to +29
links: docLinks,
};
return this.docLinks;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the client, we're only exposing the docLinks from the start contract, however on the server-side, I think there's some scenarios where consumers will need to access it at setup too, so I exposed it from both lifecycles.

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

It looks good to me! Thanks for doing this @pgayvallet!

I just added a few nits for your consideration.

packages/kbn-doc-links/README.md Show resolved Hide resolved
src/core/public/doc_links/doc_links_service.ts Outdated Show resolved Hide resolved
src/core/server/server.api.md Outdated Show resolved Hide resolved
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
core 331 334 +3

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
@kbn/doc-links - 62 +62
core 968 970 +2
total +64

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
core 132.9KB 132.9KB +6.0B

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
@kbn/doc-links - 2 +2
kibana 929 930 +1
total +3

Page load bundle

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

id before after diff
core 291.6KB 292.4KB +829.0B
Unknown metric groups

API count

id before after diff
@kbn/doc-links - 62 +62
core 2354 2361 +7
total +69

History

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

@pgayvallet pgayvallet merged commit b4e53f4 into elastic:main Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[core] docLinks server-side service
6 participants