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

[SharedUX][PoC] Move to a package architecture #127419

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .i18nrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"server": "src/legacy/server",
"share": "src/plugins/share",
"sharedUX": "src/plugins/shared_ux",
"sharedUXComponents": "packages/kbn-shared-ux-components/src",
"statusPage": "src/legacy/core_plugins/status_page",
"telemetry": ["src/plugins/telemetry", "src/plugins/telemetry_management_section"],
"timelion": ["src/plugins/vis_types/timelion"],
Expand Down
8 changes: 8 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@
"@kbn/securitysolution-utils": "link:bazel-bin/packages/kbn-securitysolution-utils",
"@kbn/server-http-tools": "link:bazel-bin/packages/kbn-server-http-tools",
"@kbn/server-route-repository": "link:bazel-bin/packages/kbn-server-route-repository",
"@kbn/shared-ux-components": "link:bazel-bin/packages/kbn-shared-ux-components",
"@kbn/shared-ux-services": "link:bazel-bin/packages/kbn-shared-ux-services",
"@kbn/shared-ux-storybook": "link:bazel-bin/packages/kbn-shared-ux-storybook",
"@kbn/shared-ux-utility": "link:bazel-bin/packages/kbn-shared-ux-utility",
"@kbn/std": "link:bazel-bin/packages/kbn-std",
"@kbn/timelion-grammar": "link:bazel-bin/packages/kbn-timelion-grammar",
"@kbn/tinymath": "link:bazel-bin/packages/kbn-tinymath",
Expand Down Expand Up @@ -196,6 +200,10 @@
"@turf/helpers": "6.0.1",
"@turf/length": "^6.0.2",
"@types/jsonwebtoken": "^8.5.6",
"@types/kbn__shared-ux-components": "link:bazel-bin/packages/kbn-shared-ux-components/npm_module_types",
"@types/kbn__shared-ux-services": "link:bazel-bin/packages/kbn-shared-ux-services/npm_module_types",
"@types/kbn__shared-ux-storybook": "link:bazel-bin/packages/kbn-shared-ux-storybook/npm_module_types",
"@types/kbn__shared-ux-utility": "link:bazel-bin/packages/kbn-shared-ux-utility/npm_module_types",
"@types/moment-duration-format": "^2.2.3",
"JSONStream": "1.3.5",
"abort-controller": "^3.0.0",
Expand Down
8 changes: 8 additions & 0 deletions packages/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ filegroup(
"//packages/kbn-securitysolution-utils:build",
"//packages/kbn-server-http-tools:build",
"//packages/kbn-server-route-repository:build",
"//packages/kbn-shared-ux-components:build",
"//packages/kbn-shared-ux-services:build",
"//packages/kbn-shared-ux-storybook:build",
"//packages/kbn-shared-ux-utility:build",
"//packages/kbn-spec-to-console:build",
"//packages/kbn-std:build",
"//packages/kbn-storybook:build",
Expand Down Expand Up @@ -138,6 +142,10 @@ filegroup(
"//packages/kbn-securitysolution-utils:build_types",
"//packages/kbn-server-http-tools:build_types",
"//packages/kbn-server-route-repository:build_types",
"//packages/kbn-shared-ux-components:build_types",
"//packages/kbn-shared-ux-services:build_types",
"//packages/kbn-shared-ux-storybook:build_types",
"//packages/kbn-shared-ux-utility:build_types",
"//packages/kbn-std:build_types",
"//packages/kbn-storybook:build_types",
"//packages/kbn-telemetry-tools:build_types",
Expand Down
149 changes: 149 additions & 0 deletions packages/kbn-shared-ux-components/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
load("@npm//@bazel/typescript:index.bzl", "ts_config")
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
load("//src/dev/bazel:index.bzl", "jsts_transpiler", "pkg_npm", "pkg_npm_types", "ts_project")

PKG_DIRNAME = "kbn-shared-ux-components"
PKG_REQUIRE_NAME = "@kbn/shared-ux-components"

SOURCE_FILES = glob(
[
"src/**/*.ts",
"src/**/*.tsx",
"src/**/*.scss",
],
exclude = [
"**/*.test.*",
],
)

SRCS = SOURCE_FILES

filegroup(
name = "srcs",
srcs = SRCS,
)

NPM_MODULE_EXTRA_FILES = [
"package.json",
]

# In this array place runtime dependencies, including other packages and NPM packages
# which must be available for this code to run.
#
# To reference other packages use:
# "//repo/relative/path/to/package"
# eg. "//packages/kbn-utils"
#
# To reference a NPM package use:
# "@npm//name-of-package"
# eg. "@npm//lodash"
RUNTIME_DEPS = [
"//packages/kbn-i18n",
"//packages/kbn-i18n-react",
"//packages/kbn-shared-ux-services",
"//packages/kbn-shared-ux-storybook",
"//packages/kbn-shared-ux-utility",
"@npm//react",
"@npm//@elastic/eui",
"@npm//@emotion/react",
"@npm//@emotion/css",
"@npm//classnames",
"@npm//@storybook/addon-actions",
]

# In this array place dependencies necessary to build the types, which will include the
# :npm_module_types target of other packages and packages from NPM, including @types/*
# packages.
#
# To reference the types for another package use:
# "//repo/relative/path/to/package:npm_module_types"
# eg. "//packages/kbn-utils:npm_module_types"
#
# References to NPM packages work the same as RUNTIME_DEPS
TYPES_DEPS = [
"//packages/kbn-i18n:npm_module_types",
"//packages/kbn-i18n-react:npm_module_types",
"//packages/kbn-shared-ux-services:npm_module_types",
"//packages/kbn-shared-ux-storybook:npm_module_types",
"//packages/kbn-shared-ux-utility:npm_module_types",
"@npm//@types/node",
"@npm//@types/jest",
"@npm//@types/react",
"@npm//@elastic/eui",
"@npm//@emotion/react",
"@npm//@emotion/css",
"@npm//@storybook/addon-actions",
"@npm//@types/classnames",
]

jsts_transpiler(
name = "target_node",
srcs = SRCS,
build_pkg_name = package_name(),
)

jsts_transpiler(
name = "target_web",
srcs = SRCS,
build_pkg_name = package_name(),
web = True,
additional_args = [
"--copy-files",
"--quiet"
],
)

ts_config(
name = "tsconfig",
src = "tsconfig.json",
deps = [
"//:tsconfig.base.json",
"//:tsconfig.bazel.json",
],
)

ts_project(
name = "tsc_types",
args = ['--pretty'],
srcs = SRCS,
deps = TYPES_DEPS,
declaration = True,
emit_declaration_only = True,
out_dir = "target_types",
root_dir = "src",
tsconfig = ":tsconfig",
)

js_library(
name = PKG_DIRNAME,
srcs = NPM_MODULE_EXTRA_FILES,
deps = RUNTIME_DEPS + [":target_node", ":target_web"],
package_name = PKG_REQUIRE_NAME,
visibility = ["//visibility:public"],
)

pkg_npm(
name = "npm_module",
deps = [":" + PKG_DIRNAME],
)

filegroup(
name = "build",
srcs = [":npm_module"],
visibility = ["//visibility:public"],
)

pkg_npm_types(
name = "npm_module_types",
srcs = SRCS,
deps = [":tsc_types"],
package_name = PKG_REQUIRE_NAME,
tsconfig = ":tsconfig",
visibility = ["//visibility:public"],
)

filegroup(
name = "build_types",
srcs = [":npm_module_types"],
visibility = ["//visibility:public"],
)
3 changes: 3 additions & 0 deletions packages/kbn-shared-ux-components/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @kbn/shared-ux-components

Empty package generated by @kbn/generate
13 changes: 13 additions & 0 deletions packages/kbn-shared-ux-components/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-shared-ux-components'],
};
8 changes: 8 additions & 0 deletions packages/kbn-shared-ux-components/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@kbn/shared-ux-components",
"private": true,
"version": "1.0.0",
"main": "./target_node/index.js",
"browser": "./target_web/index.js",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
import React from 'react';

import { EuiLoadingSpinner } from '@elastic/eui';

import { withSuspense } from '../../utility';
import { withSuspense } from '@kbn/shared-ux-utility';

export const LazyDataViewIllustration = React.lazy(() =>
import('../assets/data_view_illustration').then(({ DataViewIllustration }) => ({
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function DocumentationLink({ href }: Props) {
<EuiTitle size="xxs">
<dt className="eui-displayInline">
<FormattedMessage
id="sharedUX.noDataViews.learnMore"
id="sharedUXComponents.noDataViews.learnMore"
defaultMessage="Want to learn more?"
/>
</dt>
Expand All @@ -29,7 +29,7 @@ export function DocumentationLink({ href }: Props) {
<dd className="eui-displayInline">
<EuiLink href={href} target="_blank" external>
<FormattedMessage
id="sharedUX.noDataViews.readDocumentation"
id="sharedUXComponents.noDataViews.readDocumentation"
defaultMessage="Read the docs"
/>
</EuiLink>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface Props {
emptyPromptColor?: EuiEmptyPromptProps['color'];
}

const createDataViewText = i18n.translate('sharedUX.noDataViewsPage.addDataViewText', {
const createDataViewText = i18n.translate('sharedUXComponents.noDataViewsPage.addDataViewText', {
defaultMessage: 'Create Data View',
});

Expand Down Expand Up @@ -62,20 +62,20 @@ export const NoDataViews = ({
title={
<h2>
<FormattedMessage
id="sharedUX.noDataViews.youHaveData"
id="sharedUXComponents.noDataViews.youHaveData"
defaultMessage="You have data in Elasticsearch."
/>
<br />
<FormattedMessage
id="sharedUX.noDataViews.nowCreate"
id="sharedUXComponents.noDataViews.nowCreate"
defaultMessage="Now, create a data view."
/>
</h2>
}
body={
<p>
<FormattedMessage
id="sharedUX.noDataViews.dataViewExplanation"
id="sharedUXComponents.noDataViews.dataViewExplanation"
defaultMessage="Kibana requires a data view to identify which data streams,
indices, and index aliases you want to explore. A data view can point to a
specific index, for example, your log data from yesterday, or all indices
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
*/
import React from 'react';
import { action } from '@storybook/addon-actions';

import { docLinksServiceFactory } from '../../../services/storybook/doc_links';
import { docLinksServiceFactory } from '@kbn/shared-ux-storybook';

import { NoDataViews as NoDataViewsComponent, Props } from './no_data_views.component';
import { NoDataViews } from './no_data_views';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ import { ReactWrapper } from 'enzyme';

import { mountWithIntl } from '@kbn/test-jest-helpers';
import { EuiButton } from '@elastic/eui';
import {
SharedUXServicesProvider,
SharedUXServices,
mockServiceFactories,
} from '@kbn/shared-ux-services';

import { ServicesProvider, SharedUXServices } from '../../../services';
import { servicesFactory } from '../../../services/mocks';
import { NoDataViews } from './no_data_views';

describe('<NoDataViewsPageTest />', () => {
let services: SharedUXServices;
let mount: (element: JSX.Element) => ReactWrapper;

beforeEach(() => {
services = servicesFactory();
services = mockServiceFactories.servicesFactory();
mount = (element: JSX.Element) =>
mountWithIntl(<ServicesProvider {...services}>{element}</ServicesProvider>);
mountWithIntl(<SharedUXServicesProvider {...services}>{element}</SharedUXServicesProvider>);
});

afterEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@
*/

import React, { useCallback, useEffect, useRef } from 'react';

import { DataView } from '../../../../../data_views/public';
import { useEditors, usePermissions } from '../../../services';
import type { SharedUXEditorsService } from '../../../services/editors';
import { useEditors, usePermissions } from '@kbn/shared-ux-services';
import type { SharedUXEditorsService } from '@kbn/shared-ux-services';

import { NoDataViews as NoDataViewsComponent } from './no_data_views.component';

export interface Props {
onDataViewCreated: (dataView: DataView) => void;
onDataViewCreated: (dataView: unknown) => void;
Copy link
Contributor

@majagrubic majagrubic Mar 14, 2022

Choose a reason for hiding this comment

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

Since we're in the process of migrating to packages, having a types package is a pretty standard deal. We should really work on getting one for common types, at least the ones from data and data_view plugins. We made all this effort to migrate to typescript, only to be taking a step back now. Is this on your radar as well @spalger ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if that would accomplish more than colocating the types today. Technically, Bazel already builds a separate types "package"... and I'd be worried about keeping types too far from the relevant code.

At the same time, I'd be interested in creating a @kbn/shared-ux-types package for common and public types. I'd just want to keep it curated to truly universal/common types, rather than default to putting all types there.

It's a good point. Let's think on it a bit, see what we'd propose to put in a types package to see if there'd be value in it. @spalger @stacey-gammon I'd be curious on your input here, as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

My opinion is to stick to our '"organize by domain" philosophy and co-locate types with the thing they are describing (meaning DataView types should be next to DataView implementation stuff).

Am I understanding the issue here is that packages can't depend on plugins and thats why this had to change from a type to unknown?

I think we will eventually be able to solve this when each plugin is a package, and there is no separate packages directory at all (#112886)

cc @mistic

Copy link
Contributor

@spalger spalger Mar 14, 2022

Choose a reason for hiding this comment

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

Yeah, I feel like shared-ux is just a little ahead of the curve here. The DataView plugin will eventually be a package and @kbn/shared-ux* will be able to import the DataView type from it, a @kbn/data-views-types package, or something similar but co-located with the data views implementation stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just want to keep it curated to truly universal/common types, rather than default to putting all types there.

My opinion is to stick to our '"organize by domain" philosophy and co-locate types with the thing they are describing

Totally agree +++

dataViewsDocLink: string;
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading