[FieldFormats] Reduce `any` usage #111530

Dosant · 2021-09-08T13:19:01Z

Summary

This reduces any usage in field formats code tackling lower hanging occurrences

There is still a larger effort needed to improve types and get rid of any completely #108158. This needs more time to tackle (needs field formats public API change and then needs a bunch of downstream changes) and I am out of timeboxed time here already.

…ff-any

Dosant · 2021-09-10T14:49:35Z

src/plugins/data/common/search/search_source/search_source.ts

@@ -828,7 +828,7 @@ export class SearchSource {
    body.query = buildEsQuery(index, query, filters, esQueryConfigs);

    if (highlightAll && body.query) {
-      body.highlight = getHighlightRequest(body.query, getConfig(UI_SETTINGS.DOC_HIGHLIGHT));
+      body.highlight = getHighlightRequest(getConfig(UI_SETTINGS.DOC_HIGHLIGHT));


getHighlightRequest didn't use query inside, so I decided to remove it from the function signature to remove any.
@lukasolson, could you review this one? Maybe this wasn't correct in the first place that query is not used inside getHighlightRequest

Dosant · 2021-09-10T14:50:20Z

src/plugins/field_formats/common/converters/boolean.ts

@@ -20,7 +20,7 @@ export class BoolFormat extends FieldFormat {
  });
  static fieldType = [KBN_FIELD_TYPES.BOOLEAN, KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.STRING];

-  textConvert: TextContextTypeConvert = (value) => {
+  textConvert: TextContextTypeConvert = (value: string | number | boolean) => {


For all of these formatters, I just specified types based on what I saw is supported in runtime

Dosant · 2021-09-10T14:52:44Z

src/plugins/field_formats/common/converters/date_nanos_shared.ts

  const fracSecMatchStr = fracSecMatch ? fracSecMatch[0] : '';

  return {
    length: fracSecMatchStr.length,
    patternNanos: fracSecMatchStr,
    pattern,
-    patternEscaped: fracSecMatchStr ? pattern.replace(fracSecMatch, `[${fracSecMatch}]`) : '',
+    patternEscaped: fracSecMatchStr ? pattern.replace(fracSecMatchStr, `[${fracSecMatchStr}]`) : '',


@kertal, could you review this change please 🙏 (this was added by you in the past #43114)
when removing any typescript screamed that something was wrong here and I changed assuming this was a bug

Dosant · 2021-09-10T14:57:06Z

src/plugins/field_formats/common/field_formats_registry.ts

+    return this.getDefaultInstanceMemoized(fieldType, esTypes, params);
+  };
+
+  private getDefaultInstanceMemoized = memoize(


extracting memoized methods to not expose types from memoize publicly

elasticmachine · 2021-09-10T15:08:26Z

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

lukasolson

Code changes LGTM, didn't test

mattkime

nice work!

stratoula

Code review only. Kibana vis editors team changes LGTM!

kertal · 2021-09-13T09:51:36Z

@elasticmachine merge upstream

kertal

Data Discover LGTM, just review, only types changed

Dosant · 2021-09-13T12:05:34Z

@elasticmachine merge upstream

kibanamachine · 2021-09-13T14:24:11Z

💚 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
`fieldFormats`	233	250	+17

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`fieldFormats`	26	7	-19

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
`fieldFormats`	10	3	-7

Page load bundle

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

id	before	after	diff
`data`	777.9KB	777.8KB	-11.0B
`fieldFormats`	73.7KB	75.2KB	+1.6KB
total			+1.5KB

Unknown metric groups

API count

id	before	after	diff
`fieldFormats`	263	288	+25

History

💚 Build #152777 succeeded 226d4e5
💚 Build #152514 succeeded 83ad234
💔 Build #152502 failed bf44b50
💚 Build #152258 succeeded 7033024
💔 Build #152213 failed bb9d39207aa65c8f58c81f7cc1fae1f4609dfd00

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

kibanamachine · 2021-09-13T15:04:50Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

kibanamachine · 2021-09-13T15:05:40Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Anton Dosov <[email protected]>

Dosant added 10 commits September 8, 2021 15:15

wip reduce any in ff

4885f79

fix ts

ebe94aa

fix ts

afee3b0

fix ts

35ece5f

fix ts

2a9ca82

fix ts

3b25a6c

Merge branch 'master' of github.com:elastic/kibana into d/2021-08-26-…

e7c6d38

…ff-any

fix ts

7796c4c

wip

450ebef

Merge branch 'master' of github.com:elastic/kibana into d/2021-08-26-…

7033024

…ff-any

Dosant force-pushed the d/2021-08-26-ff-any branch from bb9d392 to 7033024 Compare September 9, 2021 14:46

Merge branch 'master' of github.com:elastic/kibana into d/2021-08-26-…

b31eb9e

…ff-any

Dosant changed the title ~~wip reduce any in ff~~ [FieldFormats] Reduce any usage Sep 10, 2021

Dosant added 2 commits September 10, 2021 13:41

remove more hidden any

bf44b50

revert

83ad234

Dosant commented Sep 10, 2021

View reviewed changes

Dosant added Feature:FieldFormatters release_note:skip Skip the PR/issue when compiling release notes Team:AppServices technical debt Improvement of the software architecture and operational architecture v7.16.0 v8.0.0 labels Sep 10, 2021

Dosant marked this pull request as ready for review September 10, 2021 15:08

Dosant requested review from a team as code owners September 10, 2021 15:08

Dosant requested review from kertal, mattkime and lukasolson September 10, 2021 15:08

lukasolson approved these changes Sep 10, 2021

View reviewed changes

mattkime approved these changes Sep 11, 2021

View reviewed changes

stratoula approved these changes Sep 13, 2021

View reviewed changes

Merge branch 'master' into d/2021-08-26-ff-any

226d4e5

kertal approved these changes Sep 13, 2021

View reviewed changes

Merge branch 'master' into d/2021-08-26-ff-any

4bfbc88

Dosant added the auto-backport Deprecated - use backport:version if exact versions are needed label Sep 13, 2021

Dosant merged commit d6badd8 into elastic:master Sep 13, 2021

kibanamachine mentioned this pull request Sep 13, 2021

[7.x] [FieldFormats] Reduce any usage (#111530) #111965

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 13, 2021

[FieldFormats] Reduce any usage (elastic#111530)

c91af0c

kibanamachine added a commit that referenced this pull request Sep 13, 2021

[FieldFormats] Reduce any usage (#111530) (#111965)

e3b8f04

Co-authored-by: Anton Dosov <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FieldFormats] Reduce `any` usage #111530

[FieldFormats] Reduce `any` usage #111530

Dosant commented Sep 8, 2021 •

edited

Loading

Dosant Sep 10, 2021

Dosant Sep 10, 2021

Dosant Sep 10, 2021

Dosant Sep 10, 2021

elasticmachine commented Sep 10, 2021

lukasolson left a comment

mattkime left a comment

stratoula left a comment

kertal commented Sep 13, 2021

kertal left a comment

Dosant commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

API count

kibanamachine commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

[FieldFormats] Reduce any usage #111530

[FieldFormats] Reduce any usage #111530

Conversation

Dosant commented Sep 8, 2021 • edited Loading

Summary

Dosant Sep 10, 2021

Choose a reason for hiding this comment

Dosant Sep 10, 2021

Choose a reason for hiding this comment

Dosant Sep 10, 2021

Choose a reason for hiding this comment

Dosant Sep 10, 2021

Choose a reason for hiding this comment

elasticmachine commented Sep 10, 2021

lukasolson left a comment

Choose a reason for hiding this comment

mattkime left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kertal commented Sep 13, 2021

kertal left a comment

Choose a reason for hiding this comment

Dosant commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Any counts in public APIs

Public APIs missing exports

Page load bundle

API count

History

kibanamachine commented Sep 13, 2021

💚 Backport successful

kibanamachine commented Sep 13, 2021

💚 Backport successful

[FieldFormats] Reduce `any` usage #111530

[FieldFormats] Reduce `any` usage #111530

Dosant commented Sep 8, 2021 •

edited

Loading