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

[kbn/io-ts] export and require importing individual functions #117958

Merged
merged 4 commits into from
Nov 9, 2021

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Nov 8, 2021

I'm trying to make it "against the rules" to import from target_node and other directories created by Bazel, and saw how we prevent importing all the code from @kbn/io-ts-utils in the browser bundles I figured we should upgrade the way we do this a bit by setting up some importable targets in the @kbn/io-ts-utils package. There is now one import target for each module/function in io-ts:

  • @kbn/io-ts-utils/deep_exact_rt
  • @kbn/io-ts-utils/iso_to_epoch_rt
  • @kbn/io-ts-utils/json_rt
  • @kbn/io-ts-utils/merge_rt
  • @kbn/io-ts-utils/non_empty_string_rt
  • @kbn/io-ts-utils/parseable_types
  • @kbn/io-ts-utils/props_to_schema
  • @kbn/io-ts-utils/strict_keys_rt
  • @kbn/io-ts-utils/to_boolean_rt
  • @kbn/io-ts-utils/to_json_schema
  • @kbn/io-ts-utils/to_number_rt

In addition to these import targets and updates to everything importing from @kbn/io-ts-utils, this adds an ESLint rule that prevents importing from the root of the @kbn/io-ts-utils package and encourages used of the other modules instead. I implemented this across both the server and public code for consistency but I'd be happy to limit this eslint rule to just the public and common code.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1184 1178 -6

Async chunks

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

id before after diff
apm 2.7MB 2.7MB -6.0KB

History

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

@spalger spalger marked this pull request as ready for review November 9, 2021 05:41
@spalger spalger requested review from a team as code owners November 9, 2021 05:41
@spalger spalger added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.16.0 v8.0.0 v8.1.0 labels Nov 9, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@spalger spalger added the release_note:skip Skip the PR/issue when compiling release notes label Nov 9, 2021
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Uptime code owner changes LGTM !!

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

Thanks @spalger, much better! Is this process scriptable?

@spalger spalger merged commit 4e443cc into elastic:main Nov 9, 2021
@spalger spalger deleted the implement/io-ts-submodules branch November 9, 2021 18:42
@spalger spalger added the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 9, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 9, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 9, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0
7.16

The backport PRs will be merged automatically after passing CI.

tylersmalley pushed a commit that referenced this pull request Nov 9, 2021
Conflict between #117958 and #115145

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
spalger pushed a commit to kibanamachine/kibana that referenced this pull request Nov 9, 2021
Conflict between elastic#117958 and elastic#115145

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
kibanamachine added a commit that referenced this pull request Nov 9, 2021
…117958) (#118074)

* [kbn/io-ts] export and require importing individual functions (#117958)

* [kbn/io-ts] fix direct import

Conflict between #117958 and #115145

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Co-authored-by: Spencer <email@spalger.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Tyler Smalley <tyler.smalley@elastic.co>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 10, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 11, 2021
kibanamachine added a commit that referenced this pull request Nov 11, 2021
…117958) (#118075)

* [kbn/io-ts] export and require importing individual functions (#117958)

* fix import

Co-authored-by: Spencer <email@spalger.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
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 release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants