diff --git a/contribute/engineering/terminology.md b/contribute/engineering/terminology.md index 5b28cb7f4b1..5392f9050e4 100644 --- a/contribute/engineering/terminology.md +++ b/contribute/engineering/terminology.md @@ -2,16 +2,14 @@ -This document defines technical terms used in Grafana. +This glossary defines technical terms used in Grafana. ## TLS/SSL The acronyms [TLS](https://en.wikipedia.org/wiki/Transport_Layer_Security) (Transport Layer Security) and -[SSL](https://en.wikipedia.org/wiki/SSL) (Secure Socket Layer) are both used to describe the HTTPS security layer, -and are in practice synonymous. However, TLS is considered the current name for the technology, and SSL is considered +[SSL](https://en.wikipedia.org/wiki/SSL) (Secure Socket Layer) are both used to describe the HTTPS security layer. +In practice, they are synonymous. However, TLS is considered the current name for the technology, and SSL is considered [deprecated](https://tools.ietf.org/html/rfc7568). -As such, while both terms are in use (also in our codebase) and are indeed interchangeable, TLS is the preferred term. -That said however, we have at Grafana Labs decided to use both acronyms in combination when referring to this type of -technology, i.e. _TLS/SSL_. This is in order to not confuse those who may not be aware of them being synonymous, -and SSL still being so prevalent in common discourse. +As such, while we use both terms in our codebase and documentation, we generally prefer TLS. +However, we use both acronyms in combination when referring to this type of technology, that is, _TLS/SSL_. We do this because we don't want to confuse readers who may not be aware of them being synonymous, and SSL is still prevalent in common discourse. diff --git a/contribute/style-guides/frontend.md b/contribute/style-guides/frontend.md index 07055362fff..c0eed80ad5d 100644 --- a/contribute/style-guides/frontend.md +++ b/contribute/style-guides/frontend.md @@ -1,41 +1,6 @@ -# Frontend Style Guide +# Frontend style guide -Generally we follow the Airbnb [React Style Guide](https://github.com/airbnb/javascript/tree/master/react). - -## Table of Contents - -- [Frontend Style Guide](#frontend-style-guide) - - - [Table of Contents](#table-of-contents) - - [Basic rules](#basic-rules) - - [Naming conventions](#naming-conventions) - - [Use `PascalCase` for:](#use-pascalcase-for) - - [Typescript class names](#typescript-class-names) - - [Types and interfaces](#types-and-interfaces) - - [Enums](#enums) - - [Use `camelCase` for:](#use-camelcase-for) - - [Functions](#functions) - - [Methods](#methods) - - [Variables](#variables) - - [React state and properties](#react-state-and-properties) - - [Emotion class names](#emotion-class-names) - - [Use `ALL_CAPS` for constants.](#use-all_caps-for-constants) - - [Use BEM convention for SASS styles.](#use-bem-convention-for-sass-styles) - - [Typing](#typing) - - [File and directory naming conventions](#file-and-directory-naming-conventions) - - [Code organization](#code-organization) - - [Exports](#exports) - - [Comments](#comments) - - [Linting](#linting) - - [React](#react) - - [Props](#props) - - [Name callback props and handlers with an "on" prefix.](#name-callback-props-and-handlers-with-an-on-prefix) - - [React Component definitions](#react-component-definitions) - - [React Component constructor](#react-component-constructor) - - [React Component defaultProps](#react-component-defaultprops) - - [State management](#state-management) - - - [Proposal for removing or replacing Angular dependencies](https://github.com/grafana/grafana/pull/23048) +Grafana Labs follows the [Airbnb React/JSX Style Guide](https://github.com/airbnb/javascript/tree/master/react) in matters pertaining to React. This guide provides highlights of the style rules we follow. ## Basic rules @@ -43,11 +8,13 @@ Generally we follow the Airbnb [React Style Guide](https://github.com/airbnb/jav - Break large components up into sub-components. - Use spaces for indentation. -### Naming conventions +## Naming conventions -#### Use `PascalCase` for: +Follow these guidelines when naming elements of your code. -##### Typescript class names +### Class names + +Use PascalCase. For example: ```typescript // bad @@ -61,7 +28,94 @@ class DataLink { } ``` -##### Types and interfaces +### Constants + +Use ALL CAPS for constants. For example: + +```typescript +// bad +const constantValue = "This string won't change"; +// bad +const constant_value = "This string won't change"; + +// good +const CONSTANT_VALUE = "This string won't change"; +``` + +### Emotion class names + +Use camelCase. For example: + +```typescript +const getStyles = (theme: GrafanaTheme2) => ({ + // bad + ElementWrapper: css`...`, + // bad + ['element-wrapper']: css`...`, + + // good + elementWrapper: css({ + padding: theme.spacing(1, 2), + background: theme.colors.background.secondary, + }), +}); +``` + +Use hook useStyles2(getStyles) to memoize the styles generation and try to avoid passing props to the getStyles function and instead compose classes using Emotion CX function. + +### Enums + +Use PascalCase. For example: + +``` +// bad +enum buttonVariant { + //... +} + +// good +enum ButtonVariant { + //... +} +``` + +### Files and directories + +Name files according to the primary export: + +- When the primary export is a class or React component, use PascalCase. +- When the primary export is a function, use camelCase. + +For files that export multiple utility functions, use the name that describes the responsibility of grouped utilities. For example, a file that exports math utilities should be named `math.ts`. + +- Use `constants.ts` for files that export constants. +- Use `actions.ts` for files that export Redux actions. +- Use `reducers.ts` for Redux reducers. +- Use `*.test.ts(x)` for test files. + +For directory names, use dash-case (sometimes called kebab-case). + +- Use `features/new-important-feature/utils.ts` + +### Functions + +Use PascalCase. For example: + +Use camelCase. + +```typescript +// bad +const CalculatePercentage = () => { ... } +// bad +const calculate_percentage = () => { ... } + +// good +const calculatePercentage = () => { ... } +``` + +### Interfaces + +Use PascalCase. For example: ``` // bad @@ -91,35 +145,11 @@ type request_info = ... type RequestInfo = ... ``` -##### Enums +### Methods -``` -// bad -enum buttonVariant { - //... -} +Use PascalCase. For example: -// good -enum ButtonVariant { - //... -} -``` - -#### Use `camelCase` for: - -##### Functions - -```typescript -// bad -const CalculatePercentage = () => { ... } -// bad -const calculate_percentage = () => { ... } - -// good -const calculatePercentage = () => { ... } -``` - -##### Methods +Use camelCase. ```typescript class DateCalculator { @@ -137,164 +167,13 @@ class DateCalculator { } ``` -##### Variables +### React components -```typescript -// bad -const QueryTargets = []; -// bad -const query_targets = []; +Follow these guidelines for naming React components. -// good -const queryTargets = []; -``` +#### React callback props and handlers -##### React state and properties - -```typescript -interface ModalState { - // bad - IsActive: boolean; - // bad - is_active: boolean; - - // good - isActive: boolean; -} -``` - -##### Emotion class names - -```typescript -const getStyles = (theme: GrafanaTheme2) => ({ - // bad - ElementWrapper: css`...`, - // bad - ['element-wrapper']: css`...`, - - // good - elementWrapper: css({ - padding: theme.spacing(1, 2), - background: theme.colors.background.secondary, - }), -}); -``` - -Use hook useStyles2(getStyles) to memoize the styles generation and try to avoid passing props to the getStyles function and instead compose classes using emotion cx function. - -#### Use `ALL_CAPS` for constants. - -```typescript -// bad -const constantValue = "This string won't change"; -// bad -const constant_value = "This string won't change"; - -// good -const CONSTANT_VALUE = "This string won't change"; -``` - -#### Use [BEM](http://getbem.com/) convention for SASS styles. - -_SASS styles are deprecated. Please migrate to Emotion whenever you need to modify SASS styles._ - -### Typing - -In general, you should let Typescript infer the types so that there's no need to explicitly define type for each variable. - -There are some exceptions to this: - -```typescript -// Typescript needs to know type of arrays or objects otherwise it would infer it as array of any - -// bad -const stringArray = []; - -// good -const stringArray: string[] = []; -``` - -Specify function return types explicitly in new code. This improves readability by being able to tell what a function returns just by looking at the signature. It also prevents errors when a function's return type is broader than expected by the author. - -> **Note:** We don't have linting for this enabled because of lots of old code that needs to be fixed first. - -```typescript -// bad -function transform(value?: string) { - if (!value) { - return undefined; - } - return applyTransform(value); -} - -// good -function transform(value?: string): TransformedValue | undefined { - if (!value) { - return undefined; - } - return applyTransform(value); -} -``` - -### File and directory naming conventions - -Name files according to the primary export: - -- When the primary export is a class or React component, use PascalCase. -- When the primary export is a function, use camelCase. - -For files exporting multiple utility functions, use the name that describes the responsibility of grouped utilities. For example, a file exporting math utilities should be named `math.ts`. - -- Use `constants.ts` for files exporting constants. -- Use `actions.ts` for files exporting Redux actions. -- Use `reducers.ts` Redux reducers. -- Use `*.test.ts(x)` for test files. - -- Use kebab case for directory names: lowercase, words delimited by hyphen ( `-` ). For example, `features/new-important-feature/utils.ts`. - -### Code organization - -Organize your code in a directory that encloses feature code: - -- Put Redux state and domain logic code in `state` directory (i.e. `features/my-feature/state/actions.ts`). -- Put React components in `components` directory (i.e. `features/my-feature/components/ButtonPeopleDreamOf.tsx`). -- Put test files next to the test subject. -- Put containers (pages) in feature root (i.e. `features/my-feature/DashboardPage.tsx`). -- Put API function calls that isn't a redux thunk in an `api.ts` file within the same directory. -- Subcomponents can live in the component folders. Small component do not need their own folder. -- Component SASS styles should live in the same folder as component code. - -For code that needs to be used by external plugin: - -- Put components and types in `@grafana/ui`. -- Put data models and data utilities in `@grafana/data`. -- Put runtime services interfaces in `@grafana/runtime`. - -#### Exports - -- Use named exports for all code you want to export from a file. -- Use declaration exports (i.e. `export const foo = ...`). -- Avoid using default exports (for example, `export default foo`). -- Export only the code that is meant to be used outside the module. - -### Comments - -- Use [TSDoc](https://github.com/microsoft/tsdoc) comments to document your code. -- Use [react-docgen](https://github.com/reactjs/react-docgen) comments (`/** ... */`) for props documentation. -- Use inline comments for comments inside functions, classes etc. -- Please try to follow the [code comment guidelines](./code-comments.md) when adding comments. - -### Linting - -Linting is performed using [@grafana/eslint-config](https://github.com/grafana/eslint-config-grafana). - -## React - -Use the following conventions when implementing React components: - -### Props - -##### Name callback props and handlers with an "on" prefix. +Name callback props and handlers with an _on_ prefix. For example: ```tsx // bad @@ -321,17 +200,9 @@ render() { ``` -##### React Component definitions +#### React component constructor -```jsx -// bad -export class YourClass extends PureComponent { ... } - -// good -export class YourClass extends PureComponent<{},{}> { ... } -``` - -##### React Component constructor +Use the following convention when implementing these React components: ```typescript // bad @@ -341,7 +212,9 @@ constructor(props) {...} constructor(props: Props) {...} ``` -##### React Component defaultProps +#### React component defaultProps + +Use the following convention when implementing these React components: ```typescript // bad @@ -351,9 +224,131 @@ static defaultProps = { ... } static defaultProps: Partial = { ... } ``` -### How to declare functional components +#### React component definitions -We prefer using function declarations over function expressions when creating a new react functional component. +Use the following convention when implementing these React components: + +```jsx +// bad +export class YourClass extends PureComponent { ... } + +// good +export class YourClass extends PureComponent<{},{}> { ... } +``` + +#### React state and properties + +Use camelCase. For example: + +```typescript +interface ModalState { + // bad + IsActive: boolean; + // bad + is_active: boolean; + + // good + isActive: boolean; +} +``` + +### SASS + +SASS styles are deprecated. You should migrate to Emotion whenever you need to modify SASS styles. + +### Types + +In general, you should let TypeScript infer the types so that there's no need to explicitly define the type for each variable. + +There are some exceptions to this: + +```typescript +// TypeScript needs to know the type of arrays or objects; otherwise, it infers type as an array of any + +// bad +const stringArray = []; + +// good +const stringArray: string[] = []; +``` + +Specify function return types explicitly in new code. This improves readability by being able to tell what a function returns just by looking at the signature. It also prevents errors when a function's return type is broader than expected by the author. + +> **Note:** Linting is not enabled for this issue because there is old code that needs to be fixed first. + +```typescript +// bad +function transform(value?: string) { + if (!value) { + return undefined; + } + return applyTransform(value); +} + +// good +function transform(value?: string): TransformedValue | undefined { + if (!value) { + return undefined; + } + return applyTransform(value); +} +``` + +### Variables + +Use PascalCase. For example: + +Use camelCase. + +```typescript +// bad +const QueryTargets = []; +// bad +const query_targets = []; + +// good +const queryTargets = []; +``` + +## Code organization + +Organize your code in a directory that encloses feature code: + +- Put Redux state and domain logic code in the `state` directory (for example, `features/my-feature/state/actions.ts`). +- Put React components in the `components` directory (for example, `features/my-feature/components/ButtonPeopleDreamOf.tsx`). +- Put test files next to the test subject. +- Put containers (pages) in the feature root (for example, `features/my-feature/DashboardPage.tsx`). +- Put API function calls that aren't a Redux thunk in an `api.ts` file within the same directory. +- Subcomponents should live in the component folders. Small components don't need their own folder. +- Component SASS styles should live in the same folder as component code. + +For code that needs to be used by an external plugin: + +- Put components and types in `@grafana/ui`. +- Put data models and data utilities in `@grafana/data`. +- Put runtime services interfaces in `@grafana/runtime`. + +### Exports + +- Use named exports for all code you want to export from a file. +- Use declaration exports (that is, `export const foo = ...`). +- Avoid using default exports (for example, `export default foo`). +- Export only the code that is meant to be used outside the module. + +### Code comments + +- Use [TSDoc](https://github.com/microsoft/tsdoc) comments to document your code. +- Use [react-docgen](https://github.com/reactjs/react-docgen) comments (`/** ... */`) for props documentation. +- Use inline comments for comments inside functions, classes, etc. +- Please try to follow the [code comment guidelines](./code-comments.md) when adding comments. + +## Linting + +Linting is performed using [@grafana/eslint-config](https://github.com/grafana/eslint-config-grafana). + +## Functional components + +Use function declarations instead of function expressions when creating a new React functional component. For example: ```typescript // bad @@ -366,13 +361,6 @@ export const Component: React.FC = (props) => { ... } export function Component(props: Props) { ... } ``` -Some interesting readings on the topic: - -- [Create React App: Remove React.FC from typescript template](https://github.com/facebook/create-react-app/pull/8177) -- [Kent C. Dodds: How to write a React Component in Typescript](https://kentcdodds.com/blog/how-to-write-a-react-component-in-typescript) -- [Kent C. Dodds: Function forms](https://kentcdodds.com/blog/function-forms) -- [Sam Hendrickx: Why you probably shouldn't use React.FC?](https://medium.com/raccoons-group/why-you-probably-shouldnt-use-react-fc-to-type-your-react-components-37ca1243dd13) - ## State management - Don't mutate state in reducers or thunks. diff --git a/contribute/style-guides/redux.md b/contribute/style-guides/redux.md index 5952b706ddf..85c59859669 100644 --- a/contribute/style-guides/redux.md +++ b/contribute/style-guides/redux.md @@ -2,16 +2,20 @@ Grafana uses [Redux Toolkit](https://redux-toolkit.js.org/) to handle Redux boilerplate code. -> Some of our Reducers are used by Angular and therefore state is to be considered as mutable for those reducers. +> **Note:** Some of our reducers are used by Angular; therefore, consider state to be mutable for those reducers. ## Test functionality +Here's how to test the functioning of your Redux reducers. + ### reducerTester -Fluent API that simplifies the testing of reducers +Use the Fluent API framework to simplify the testing of reducers. #### Usage +Example of `reducerTester` in use: + ```typescript reducerTester() .givenReducer(someReducer, initialState) @@ -21,9 +25,9 @@ reducerTester() #### Complex usage -Sometimes you encounter a `resulting state` that contains properties that are hard to compare, such as `Dates`, but you still want to compare that other props in state are correct. +Sometimes you encounter a _resulting state_ that contains properties that are hard to compare, such as `Dates`, but you still want to evaluate whether other props in state are correct. -Then you can use `thenStatePredicateShouldEqual` function on `reducerTester` that will return the `resulting state` so that you can expect upon individual properties.. +In these cases, you can evaluate individual properties by using `thenStatePredicateShouldEqual` function on `reducerTester` that will return the resulting state. For example: ```typescript reducerTester() @@ -37,10 +41,12 @@ reducerTester() ### thunkTester -Fluent API that simplifies the testing of thunks. +Here's a Fluent API function that simplifies the testing of thunks. #### Usage +Example of `thunkTester` in use: + ```typescript const dispatchedActions = await thunkTester(initialState).givenThunk(someThunk).whenThunkIsDispatched(arg1, arg2, arg3); @@ -49,7 +55,7 @@ expect(dispatchedActions).toEqual([someAction('reducer tests')]); ## Typing of connected props -It is possible to infer connected props automatically from `mapStateToProps` and `mapDispatchToProps` using a helper type `ConnectedProps` from Redux. For this to work the `connect` call has to be split into two parts. +It is possible to infer connected props automatically from `mapStateToProps` and `mapDispatchToProps` using a helper type `ConnectedProps` from Redux. For this to work properly, split the `connect` call into two parts like so: ```typescript import { connect, ConnectedProps } from 'react-redux'; @@ -80,4 +86,4 @@ class PanelEditorUnconnected extends PureComponent {} export const PanelEditor = connector(PanelEditorUnconnected); ``` -For more examples, refer to the [Redux docs](https://react-redux.js.org/using-react-redux/static-typing#inferring-the-connected-props-automatically). +For more examples, refer to the [Redux documentation](https://react-redux.js.org/using-react-redux/static-typing#inferring-the-connected-props-automatically). diff --git a/contribute/style-guides/testing.md b/contribute/style-guides/testing.md index 18e4c13e248..18ae9b27be8 100644 --- a/contribute/style-guides/testing.md +++ b/contribute/style-guides/testing.md @@ -1,10 +1,11 @@ -# Testing Guidelines +# Testing guidelines The goal of this document is to address the most frequently asked "How to" questions related to unit testing. -## Best practices +## Some recommended practices for testing -- Default to the `*ByRole` queries when testing components as it encourages testing with accessibility concerns in mind. It's also possible to use `*ByLabelText` queries. However, the `*ByRole` queries are [more robust](https://testing-library.com/docs/queries/bylabeltext/#name) and are generally recommended over the former. +- Default to the `*ByRole` queries when testing components because it encourages testing with accessibility concerns in mind. +- Alternatively, you could use `*ByLabelText` queries for testing components. However, we recommend the `*ByRole` queries because they are [more robust](https://testing-library.com/docs/queries/bylabeltext/#name). ## Testing User Interactions @@ -13,7 +14,7 @@ We use the [user-event](https://testing-library.com/docs/user-event/intro) libra There are two important considerations when working with `userEvent`: 1. All methods in `userEvent` are asynchronous, and thus require the use of `await` when called. -2. Directly calling methods from `userEvent` may not be supported in future versions. As such, it's necessary to first call `userEvent.setup()` prior to the tests. This method returns a `userEvent` instance, complete with all its methods. This setup process can be simplified using a utility function: +1. Directly calling methods from `userEvent` may not be supported in future versions. As such, it's necessary to first call `userEvent.setup()` prior to the tests. This method returns a `userEvent` instance, complete with all its methods. This setup process can be simplified using a utility function: ```tsx import { render, screen } from '@testing-library/react'; @@ -32,15 +33,15 @@ it('should render', async () => { }); ``` -## Debugging Tests +## Debug tests There are a few utilities that can be useful for debugging tests: -- [screen.debug()](https://testing-library.com/docs/queries/about/#screendebug) - This function prints a human-readable representation of the document's DOM tree when called without arguments, or the DOM tree of specific node(s) when provided with arguments. It is internally using `console.log` to log the output to terminal. -- [Testing Playground](https://testing-playground.com/) - An interactive sandbox that allows testing which queries work with specific HTML elements. -- [logRoles](https://testing-library.com/docs/dom-testing-library/api-debugging/#prettydom) - A utility function that prints out all the implicit ARIA roles for a given DOM tree. +- [screen.debug()](https://testing-library.com/docs/queries/about/#screendebug) - This function prints a human-readable representation of the document's DOM tree when called without arguments, or the DOM tree of specific node or nodes when provided with arguments. It is internally using `console.log` to log the output to terminal. +- [Testing Playground](https://testing-playground.com/) - An interactive sandbox that allows testing of which queries work with specific HTML elements. +- [prettyDOM logRoles](https://testing-library.com/docs/dom-testing-library/api-debugging/#prettydom) - A utility function that prints out all the implicit ARIA roles for a given DOM tree. -## Testing Select Components +## Testing select components Here, the [OrgRolePicker](https://github.com/grafana/grafana/blob/38863844e7ac72c7756038a1097f89632f9985ff/public/app/features/admin/OrgRolePicker.tsx) component is used as an example. This component essentially serves as a wrapper for the `Select` component, complete with its own set of options. @@ -78,7 +79,7 @@ export function OrgRolePicker({ value, onChange, 'aria-label': ariaLabel, inputI ### Querying the Select Component -The recommended way to query `Select` components is by using a label. Add a `label` element and provide the `htmlFor` prop with a matching `inputId`. Alternatively, `aria-label` can be specified on the `Select`. +It is a recommended practice to query `Select` components by using a label. Add a `label` element and provide the `htmlFor` prop with a matching `inputId`. Alternatively, you can specify `aria-label` on the `Select` statement. ```tsx describe('OrgRolePicker', () => { @@ -94,7 +95,7 @@ describe('OrgRolePicker', () => { }); ``` -### Testing the Display of Correct Options +### Test the display of correct options At times, it might be necessary to verify that the `Select` component is displaying the correct options. In such instances, the best solution is to click the `Select` component and match the desired option using the `*ByText` query. @@ -129,11 +130,11 @@ it('should select an option', async () => { }); ``` -## Mocking Objects and Functions +## Mock objects and functions -### Mocking the `window` Object and Its Methods +### Mock the `window` object and its methods -The recommended approach for mocking the `window` object is to use Jest spies. Jest's spy functions provide a built-in mechanism for restoring mocks. This feature eliminates the need to manually save a reference to the `window` object. +The recommended approach for mocking the `window` object is to use [Jest spies](https://jestjs.io/docs/jest-object). Jest's spy functions provide a built-in mechanism for restoring mocks. This feature eliminates the need to manually save a reference to the `window` object. ```tsx let windowSpy: jest.SpyInstance; @@ -156,7 +157,7 @@ it('should test with window', function () { ### Mocking getBackendSrv() -The `getBackendSrv()` function is used to make HTTP requests to the Grafana backend. It is possible to mock this function using the `jest.mock` method. +Use the `getBackendSrv()` function to make HTTP requests to the Grafana backend. It is possible to mock this function using the `jest.mock` method. ```tsx jest.mock('@grafana/runtime', () => ({ @@ -169,9 +170,9 @@ jest.mock('@grafana/runtime', () => ({ #### Mocking getBackendSrv for AsyncSelect -The `AsyncSelect` component is used to asynchronously load options. As such, it often relies on the `getBackendSrv` for loading the options. +Use the `AsyncSelect` component to asynchronously load options. This component often relies on the `getBackendSrv` for loading the options. -Here's how the test would look like for this [OrgPicker](https://github.com/grafana/grafana/blob/38863844e7ac72c7756038a1097f89632f9985ff/public/app/core/components/Select/OrgPicker.tsx) component, which uses `AsyncSelect` under the hood. +Here's what the test looks like for this [OrgPicker](https://github.com/grafana/grafana/blob/38863844e7ac72c7756038a1097f89632f9985ff/public/app/core/components/Select/OrgPicker.tsx) component, which uses `AsyncSelect` under the hood: ```tsx import { screen, render } from '@testing-library/react';