Docs: Edit contribute/style-guides (part 11 of doc improvement project) (#92169)

* Docs: edit contribute/style-guides

* Add missing 'is'

* Improve grammar of code comment

* Prettier fixes

* Minor fix

* Minor fix

---------

Co-authored-by: Jack Baldry <jack.baldry@grafana.com>
pull/92346/head^2
Joseph Perez 10 months ago committed by GitHub
parent ca66133636
commit 8cc95a7459
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 12
      contribute/engineering/terminology.md
  2. 390
      contribute/style-guides/frontend.md
  3. 20
      contribute/style-guides/redux.md
  4. 35
      contribute/style-guides/testing.md

@ -2,16 +2,14 @@
<!-- Keep terms in alphabetical order: -->
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.

@ -1,41 +1,6 @@
# 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)
# Frontend style guide
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
Follow these guidelines when naming elements of your code.
#### Use `PascalCase` for:
### Class names
##### Typescript class names
Use PascalCase. For example:
```typescript
// bad
@ -61,37 +28,44 @@ class DataLink {
}
```
##### Types and interfaces
### Constants
```
// bad
interface buttonProps {
//...
}
Use ALL CAPS for constants. For example:
```typescript
// bad
interface button_props {
//...
}
const constantValue = "This string won't change";
// bad
interface IButtonProps {
//...
}
const constant_value = "This string won't change";
// good
interface ButtonProps {
//...
}
const CONSTANT_VALUE = "This string won't change";
```
// bad
type requestInfo = ...
// bad
type request_info = ...
### Emotion class names
// good
type RequestInfo = ...
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,
}),
});
```
##### Enums
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
@ -105,9 +79,29 @@ enum ButtonVariant {
}
```
#### Use `camelCase` for:
### 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).
##### Functions
- Use `features/new-important-feature/utils.ts`
### Functions
Use PascalCase. For example:
Use camelCase.
```typescript
// bad
@ -119,7 +113,43 @@ const calculate_percentage = () => { ... }
const calculatePercentage = () => { ... }
```
##### Methods
### Interfaces
Use PascalCase. For example:
```
// bad
interface buttonProps {
//...
}
// bad
interface button_props {
//...
}
// bad
interface IButtonProps {
//...
}
// good
interface ButtonProps {
//...
}
// bad
type requestInfo = ...
// bad
type request_info = ...
// good
type RequestInfo = ...
```
### Methods
Use PascalCase. For example:
Use camelCase.
```typescript
class DateCalculator {
@ -137,75 +167,103 @@ class DateCalculator {
}
```
##### Variables
### React components
```typescript
// bad
const QueryTargets = [];
Follow these guidelines for naming React components.
#### React callback props and handlers
Name callback props and handlers with an _on_ prefix. For example:
```tsx
// bad
const query_targets = [];
handleChange = () => {
// good
const queryTargets = [];
```
};
##### React state and properties
render() {
return (
<MyComponent changed={this.handleChange} />
);
}
```typescript
interface ModalState {
// bad
IsActive: boolean;
// bad
is_active: boolean;
// good
onChange = () => {
// good
isActive: boolean;
};
render() {
return (
<MyComponent onChange={this.onChange} />
);
}
```
##### Emotion class names
#### React component constructor
Use the following convention when implementing these React components:
```typescript
const getStyles = (theme: GrafanaTheme2) => ({
// bad
ElementWrapper: css`...`,
// bad
['element-wrapper']: css`...`,
// bad
constructor(props) {...}
// good
elementWrapper: css({
padding: theme.spacing(1, 2),
background: theme.colors.background.secondary,
}),
});
// good
constructor(props: Props) {...}
```
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.
#### React component defaultProps
#### Use `ALL_CAPS` for constants.
Use the following convention when implementing these React components:
```typescript
// bad
const constantValue = "This string won't change";
static defaultProps = { ... }
// good
static defaultProps: Partial<Props> = { ... }
```
#### React component definitions
Use the following convention when implementing these React components:
```jsx
// bad
const constant_value = "This string won't change";
export class YourClass extends PureComponent { ... }
// good
const CONSTANT_VALUE = "This string won't change";
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;
}
```
#### Use [BEM](http://getbem.com/) convention for SASS styles.
### SASS
_SASS styles are deprecated. Please migrate to Emotion whenever you need to modify SASS styles._
SASS styles are deprecated. You should migrate to Emotion whenever you need to modify SASS styles.
### Typing
### Types
In general, you should let Typescript infer the types so that there's no need to explicitly define type for each variable.
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 type of arrays or objects otherwise it would infer it as array of any
// TypeScript needs to know the type of arrays or objects; otherwise, it infers type as an array of any
// bad
const stringArray = [];
@ -216,7 +274,7 @@ 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.
> **Note:** Linting is not enabled for this issue because there is old code that needs to be fixed first.
```typescript
// bad
@ -236,124 +294,61 @@ function transform(value?: string): TransformedValue | undefined {
}
```
### File and directory naming conventions
Name files according to the primary export:
### Variables
- When the primary export is a class or React component, use PascalCase.
- When the primary export is a function, use camelCase.
Use PascalCase. For example:
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 camelCase.
- 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.
```typescript
// bad
const QueryTargets = [];
// bad
const query_targets = [];
- Use kebab case for directory names: lowercase, words delimited by hyphen ( `-` ). For example, `features/new-important-feature/utils.ts`.
// good
const queryTargets = [];
```
### Code organization
## 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 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 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.
- 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 external plugin:
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
### Exports
- Use named exports for all code you want to export from a file.
- Use declaration exports (i.e. `export const foo = ...`).
- 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.
### Comments
### 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.
- 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
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.
```tsx
// bad
handleChange = () => {
};
render() {
return (
<MyComponent changed={this.handleChange} />
);
}
// good
onChange = () => {
## Functional components
};
render() {
return (
<MyComponent onChange={this.onChange} />
);
}
```
##### React Component definitions
```jsx
// bad
export class YourClass extends PureComponent { ... }
// good
export class YourClass extends PureComponent<{},{}> { ... }
```
##### React Component constructor
```typescript
// bad
constructor(props) {...}
// good
constructor(props: Props) {...}
```
##### React Component defaultProps
```typescript
// bad
static defaultProps = { ... }
// good
static defaultProps: Partial<Props> = { ... }
```
### How to declare functional components
We prefer using function declarations over function expressions when creating a new react functional component.
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> = (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.

@ -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<Props> {}
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).

@ -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';

Loading…
Cancel
Save