Skip to content

Create server-side docLinks service#123818

Merged
pgayvallet merged 19 commits intoelastic:mainfrom
pgayvallet:kbn-95389-server-side-docLinks
Feb 1, 2022
Merged

Create server-side docLinks service#123818
pgayvallet merged 19 commits intoelastic:mainfrom
pgayvallet:kbn-95389-server-side-docLinks

Conversation

@pgayvallet
Copy link
Copy Markdown
Contributor

@pgayvallet pgayvallet commented Jan 26, 2022 •
edited
Loading

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 Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.1.0 release_note:skip Skip the PR/issue when compiling release notes labels Jan 27, 2022
@pgayvallet pgayvallet mentioned this pull request Jan 27, 2022
Merged
@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
Copy Markdown
Contributor

elasticmachine commented Jan 27, 2022

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

pgayvallet
pgayvallet commented Jan 27, 2022
View reviewed changes
src/core/server/doc_links/doc_links_service.ts
Comment on lines +27 to +29
links: docLinks,
};
return this.docLinks;
Copy link
Copy Markdown
Contributor Author

@pgayvallet pgayvallet Jan 27, 2022

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.

afharo
afharo approved these changes Jan 31, 2022
View reviewed changes
Copy link
Copy Markdown
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.

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Feb 1, 2022

💚 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
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Feb 1, 2022
@lcawl lcawl mentioned this pull request Feb 1, 2022
Closed
@qn895 qn895 mentioned this pull request Feb 1, 2022
Merged
1 task
@tsullivan tsullivan mentioned this pull request Feb 4, 2022
Closed
@TinaHeiligers TinaHeiligers mentioned this pull request Mar 3, 2022
Closed
@lcawl lcawl mentioned this pull request Sep 22, 2023
Closed
@dedemorton dedemorton mentioned this pull request Feb 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcawl lcawl lcawl left review comments

@afharo afharo afharo approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[core] docLinks server-side service

6 participants

@pgayvallet @elasticmachine @kibana-ci @afharo @lcawl @kibanamachine