Explicit namespaces for esQuery and esKuery by lizozom · Pull Request #57172 · elastic/kibana

lizozom · 2020-02-09T16:35:16Z

Summary

This PR applies the proposed resolution for #52374, trying to balance code readability, API discoverability, DX and documentation generation using ApiExtractor on the esQuery and esKuery namespaces of data plugin.

The solution involves:

Exporting types directly from index.ts, with a prefix (consistent with core)
Exporting utility functions and constants using a "namespace" (A simple object).

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-02-09T16:35:19Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

lizozom · 2020-02-09T16:37:12Z

@elasticmachine merge upstream

…izozom/kibana into newplatform/data/kuery-query-namespace

pgayvallet

LGTM for platform changes

ppisljar

code LGTM

lizozom · 2020-02-10T12:51:43Z

@XavierM

SIEM is not marked as code owners (and I don;t know your GITHUB handle), so you didn't get an automatic notification, but I wanted to ket you know about some changes in this PR:

I removed the copy of x-pack/legacy/plugins/siem/common/typed_json.ts you had, as its now exported from kibana_utils
Some places in import from deep folders withing a folder (i.e. src/plugins/data/common/es_query) or from the common folder. Imports should be made from the top level of a plugin, and from public or server , so I fixed a couple of those as well.

…ata/kuery-query-namespace

FieldFormatsRegistry,

lukeelmers

This feels SO much cleaner than what we had before! Thanks for taking the time to do this @lizozom ... Code all LGTM

lukeelmers · 2020-02-10T16:17:01Z

src/plugins/kibana_utils/common/typed_json.ts

+export type JsonValue = null | boolean | number | string | JsonObject | JsonArray;
+
+export interface JsonObject {
+  [key: string]: JsonValue;
+}
+
+// eslint-disable-next-line @typescript-eslint/no-empty-interface
+export interface JsonArray extends Array<JsonValue> {}


Would it make more sense to put this in packages/kbn-utility-types?

I could go either way, but this does feel like a pretty basic TS interface that I could see folks reaching for a lot, and kbn-utility-types seems designed to address this use case more than kibana_utils.

…kuery-query-namespace

lizozom · 2020-02-10T16:36:48Z

x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/index.tsx

 import { Dispatch } from 'redux';
 import { ActionCreator } from 'typescript-fsa';

-import { esFilters, esQuery } from '../../../../../../../../../src/plugins/data/common/es_query';


@XavierM FYI

lizozom · 2020-02-10T16:37:10Z

x-pack/legacy/plugins/graph/public/types/workspace_state.ts

 import { WorkspaceField, AdvancedSettings } from './app_state';
-
-// eslint-disable-next-line @typescript-eslint/no-empty-interface
-interface JsonArray extends Array<JsonValue> {}


@flash1293 FYI

kibanamachine · 2020-02-10T18:01:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e6a9c0c

History

💚 Build #25610 succeeded de84b67
💔 Build #25582 failed 7b55925
💚 Build #25449 succeeded 3dee295
💔 Build #25445 failed fe08d7a

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

flash1293

Graph changes LGTM, thanks for the improvement 👍

* Explicit namespaces for esQuery and esQuery * Remove unnecessary file from siem * remove jsonvalue definition from siem * server FieldFormatsRegistry, Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Explicit namespaces for esQuery and esQuery * Remove unnecessary file from siem * remove jsonvalue definition from siem * server FieldFormatsRegistry, Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Liza K added 2 commits February 9, 2020 18:29

Explicit namespaces for esQuery and esQuery

7c5636b

Remove unnecessary file from siem

bc01a5b

lizozom added Feature:New Platform v8.0.0 Team:AppArch v7.7.0 labels Feb 9, 2020

lizozom self-assigned this Feb 9, 2020

lizozom added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Feb 9, 2020

lizozom changed the title ~~Explicit namespaces for esQuery and esQuery~~ Explicit namespaces for esQuery and esKuery Feb 9, 2020

elasticmachine and others added 3 commits February 9, 2020 10:37

Merge branch 'master' into newplatform/data/kuery-query-namespace

fe08d7a

remove jsonvalue definition from siem

c1c5193

Merge branch 'newplatform/data/kuery-query-namespace' of github.com:l…

3dee295

…izozom/kibana into newplatform/data/kuery-query-namespace

lizozom marked this pull request as ready for review February 9, 2020 20:34

lizozom requested a review from a team February 9, 2020 20:34

lizozom requested review from a team as code owners February 9, 2020 20:34

lizozom added the review label Feb 9, 2020

pgayvallet approved these changes Feb 10, 2020

View reviewed changes

lizozom requested a review from ppisljar February 10, 2020 09:21

lizozom mentioned this pull request Feb 10, 2020

AppArch plugins documentation generation #56881

Closed

11 tasks

lizozom added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Feb 10, 2020

ppisljar approved these changes Feb 10, 2020

View reviewed changes

Liza K added 2 commits February 10, 2020 17:03

Merge branch 'master' of github.com:elastic/kibana into newplatform/d…

7b55925

…ata/kuery-query-namespace

server

de84b67

FieldFormatsRegistry,

lukeelmers approved these changes Feb 10, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into newplatform/data/…

e6a9c0c

…kuery-query-namespace

lizozom commented Feb 10, 2020

View reviewed changes

flash1293 approved these changes Feb 11, 2020

View reviewed changes

lizozom merged commit eb54678 into elastic:master Feb 11, 2020

lizozom mentioned this pull request Feb 11, 2020

[7.x] Explicit namespaces for esQuery and esKuery (#57172) #57297

Merged

alexwizp mentioned this pull request Feb 12, 2020

fix Master branch #57421

Merged

7 tasks

lizozom mentioned this pull request Feb 12, 2020

Explicit exports for data plugin #57434

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explicit namespaces for esQuery and esKuery#57172

Explicit namespaces for esQuery and esKuery#57172
lizozom merged 8 commits intoelastic:masterfrom
lizozom:newplatform/data/kuery-query-namespace

lizozom commented Feb 9, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Feb 9, 2020

Uh oh!

lizozom commented Feb 9, 2020

Uh oh!

pgayvallet left a comment

Uh oh!

ppisljar left a comment

Uh oh!

lizozom commented Feb 10, 2020

Uh oh!

lukeelmers left a comment

Uh oh!

lukeelmers Feb 10, 2020

Uh oh!

lizozom Feb 10, 2020

Uh oh!

lizozom Feb 10, 2020

Uh oh!

kibanamachine commented Feb 10, 2020

Uh oh!

flash1293 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

lizozom commented Feb 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Feb 9, 2020

Uh oh!

lizozom commented Feb 9, 2020

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

lizozom commented Feb 10, 2020

Uh oh!

lukeelmers left a comment

Choose a reason for hiding this comment

Uh oh!

lukeelmers Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

lizozom Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

lizozom Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 10, 2020

💚 Build Succeeded

History

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lizozom commented Feb 9, 2020 •

edited

Loading