Skip to content

Commit

Permalink
Merge branch 'master' of github.com:elastic/kibana into CIT_For_INP_F…
Browse files Browse the repository at this point in the history
…ail_Processor
  • Loading branch information
John Dorlus committed May 9, 2021
2 parents 1ae1d3d + f669add commit 377deac
Show file tree
Hide file tree
Showing 63 changed files with 854 additions and 690 deletions.
66 changes: 38 additions & 28 deletions STYLEGUIDE.md → STYLEGUIDE.mdx
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# Kibana Style Guide
---
id: kibStyleGuide
slug: /kibana-dev-docs/styleguide
title: StyleGuide
summary: JavaScript/TypeScript styleguide.
date: 2021-05-06
tags: ['kibana', 'onboarding', 'dev', 'styleguide', 'typescript', 'javascript']
---

This guide applies to all development within the Kibana project and is
recommended for the development of all Kibana plugins.

- [General](#general)
- [HTML](#html)
- [API endpoints](#api-endpoints)
- [TypeScript/JavaScript](#typeScript/javaScript)
- [SASS files](#sass-files)
- [React](#react)

Besides the content in this style guide, the following style guides may also apply
to all development within the Kibana project. Please make sure to also read them:

Expand Down Expand Up @@ -52,9 +52,7 @@ This part contains style guide rules around general (framework agnostic) HTML us
Use camel case for the values of attributes such as `id` and `data-test-subj` selectors.

```html
<button id="veryImportantButton" data-test-subj="clickMeButton">
Click me
</button>
<button id="veryImportantButton" data-test-subj="clickMeButton">Click me</button>
```

The only exception is in cases where you're dynamically creating the value, and you need to use
Expand Down Expand Up @@ -378,6 +376,20 @@ import inFoo from 'foo/child';
import inSibling from '../foo/child';
```
#### Avoid export \* in top level index.ts files
The exports in `common/index.ts`, `public/index.ts` and `server/index.ts` dictate a plugin's public API. The public API should be carefully controlled, and using `export *` makes it very easy for a developer working on internal changes to export a new public API unintentionally.
```js
// good
export { foo } from 'foo';
export { child } from './child';

// bad
export * from 'foo/child';
export * from '../foo/child';
```
### Global definitions
Don't do this. Everything should be wrapped in a module that can be depended on
Expand Down Expand Up @@ -592,20 +604,20 @@ Do not use setters, they cause more problems than they can solve.
### Avoid circular dependencies
As part of a future effort to use correct and idempotent build tools we need our code to be
able to be represented as a directed acyclic graph. We must avoid having circular dependencies
both on code and type imports to achieve that. One of the most critical parts is the plugins
code. We've developed a tool to identify plugins with circular dependencies which
has allowed us to build a list of plugins who have circular dependencies
between each other.
When building plugins we should avoid importing from plugins
who are known to have circular dependencies at the moment as well as introducing
new circular dependencies. You can run the same tool we use on our CI locally by
typing `node scripts/find_plugins_with_circular_deps --debug`. It will error out in
case new circular dependencies has been added with your changes
able to be represented as a directed acyclic graph. We must avoid having circular dependencies
both on code and type imports to achieve that. One of the most critical parts is the plugins
code. We've developed a tool to identify plugins with circular dependencies which
has allowed us to build a list of plugins who have circular dependencies
between each other.
When building plugins we should avoid importing from plugins
who are known to have circular dependencies at the moment as well as introducing
new circular dependencies. You can run the same tool we use on our CI locally by
typing `node scripts/find_plugins_with_circular_deps --debug`. It will error out in
case new circular dependencies has been added with your changes
(which will also happen in the CI) as well as print out the current list of
the known circular dependencies which, as mentioned before, should not be imported
by your code until the circular dependencies on these have been solved.
the known circular dependencies which, as mentioned before, should not be imported
by your code until the circular dependencies on these have been solved.
## SASS files
Expand All @@ -626,10 +638,8 @@ import './component.scss';
// All other imports below the SASS import

export const Component = () => {
return (
<div className="plgComponent" />
);
}
return <div className="plgComponent" />;
};
```
```scss
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@
"proxy-from-env": "1.0.0",
"proxyquire": "1.8.0",
"puid": "1.0.7",
"puppeteer": "npm:@elastic/[email protected]",
"puppeteer": "^8.0.0",
"query-string": "^6.13.2",
"raw-loader": "^3.1.0",
"rbush": "^3.0.1",
Expand Down Expand Up @@ -586,7 +586,6 @@
"@types/pretty-ms": "^5.0.0",
"@types/prop-types": "^15.7.3",
"@types/proper-lockfile": "^3.0.1",
"@types/puppeteer": "^5.4.1",
"@types/rbush": "^3.0.0",
"@types/reach__router": "^1.2.6",
"@types/react": "^16.9.36",
Expand Down
1 change: 0 additions & 1 deletion src/core/server/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { SavedObjectsClientContract } from '../saved_objects/types';
import { UiSettingsParams, UserProvidedValues, PublicUiSettingsParams } from '../../types';
export type {
Expand Down
9 changes: 9 additions & 0 deletions src/core/server/ui_settings/ui_settings_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,15 @@ describe('ui settings', () => {
bar: 'user-provided',
});
});

it('throws if mutates the result of getAll()', async () => {
const { uiSettings } = setup({ esDocSource: {} });
const result = await uiSettings.getAll();

expect(() => {
result.foo = 'bar';
}).toThrow();
});
});

describe('#get()', () => {
Expand Down
36 changes: 19 additions & 17 deletions src/core/server/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { defaultsDeep, omit } from 'lodash';
import { omit } from 'lodash';

import { SavedObjectsErrorHelpers } from '../saved_objects';
import { SavedObjectsClientContract } from '../saved_objects/types';
Expand Down Expand Up @@ -35,10 +35,7 @@ interface UserProvidedValue<T = unknown> {
isOverridden?: boolean;
}

type UiSettingsRawValue = UiSettingsParams & UserProvidedValue;

type UserProvided<T = unknown> = Record<string, UserProvidedValue<T>>;
type UiSettingsRaw = Record<string, UiSettingsRawValue>;

export class UiSettingsClient implements IUiSettingsClient {
private readonly type: UiSettingsServiceOptions['type'];
Expand All @@ -47,6 +44,7 @@ export class UiSettingsClient implements IUiSettingsClient {
private readonly savedObjectsClient: UiSettingsServiceOptions['savedObjectsClient'];
private readonly overrides: NonNullable<UiSettingsServiceOptions['overrides']>;
private readonly defaults: NonNullable<UiSettingsServiceOptions['defaults']>;
private readonly defaultValues: Record<string, unknown>;
private readonly log: Logger;
private readonly cache: Cache;

Expand All @@ -56,10 +54,15 @@ export class UiSettingsClient implements IUiSettingsClient {
this.id = id;
this.buildNum = buildNum;
this.savedObjectsClient = savedObjectsClient;
this.defaults = defaults;
this.overrides = overrides;
this.log = log;
this.cache = new Cache();
this.defaults = defaults;
const defaultValues: Record<string, unknown> = {};
Object.keys(this.defaults).forEach((key) => {
defaultValues[key] = this.defaults[key].value;
});
this.defaultValues = defaultValues;
}

getRegistered() {
Expand All @@ -72,17 +75,21 @@ export class UiSettingsClient implements IUiSettingsClient {

async get<T = any>(key: string): Promise<T> {
const all = await this.getAll();
return all[key];
return all[key] as T;
}

async getAll<T = any>() {
const raw = await this.getRaw();
const result = { ...this.defaultValues };

return Object.keys(raw).reduce((all, key) => {
const item = raw[key];
all[key] = ('userValue' in item ? item.userValue : item.value) as T;
return all;
}, {} as Record<string, T>);
const userProvided = await this.getUserProvided();
Object.keys(userProvided).forEach((key) => {
if (userProvided[key].userValue !== undefined) {
result[key] = userProvided[key].userValue;
}
});

Object.freeze(result);
return result as Record<string, T>;
}

async getUserProvided<T = unknown>(): Promise<UserProvided<T>> {
Expand Down Expand Up @@ -142,11 +149,6 @@ export class UiSettingsClient implements IUiSettingsClient {
}
}

private async getRaw(): Promise<UiSettingsRaw> {
const userProvided = await this.getUserProvided();
return defaultsDeep({}, userProvided, this.defaults);
}

private validateKey(key: string, value: unknown) {
const definition = this.defaults[key];
if (value === null || definition === undefined) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ export class FieldFormatsService {
return {
fieldFormatServiceFactory: async (uiSettings: IUiSettingsClient) => {
const fieldFormatsRegistry = new FieldFormatsRegistry();
const uiConfigs = await uiSettings.getAll();
const coreUiConfigs = await uiSettings.getAll();
const registeredUiSettings = uiSettings.getRegistered();
const uiConfigs = { ...coreUiConfigs };

Object.keys(registeredUiSettings).forEach((key) => {
if (has(uiConfigs, key) && registeredUiSettings[key].type === 'json') {
Expand Down
Loading

0 comments on commit 377deac

Please sign in to comment.