Commit ed1b515f by ryan

Merge remote-tracking branch 'grafana/master' into table-reducer

* grafana/master:
  docs: renamed file and added redux framework file
  docs: moved examples to frontend.md
  docs: intial draft for frontend review doc
  Use ora#fail instead of console.log
  Remove .only function
  Add more patterns to no-only-test task
parents a0fa5698 8859d16a
......@@ -161,7 +161,7 @@ describe('change threshold value', () => {
});
describe('on blur threshold value', () => {
it.only('should resort rows and update indexes', () => {
it('should resort rows and update indexes', () => {
const { instance } = setup();
const thresholds = [
{ index: 0, value: -Infinity, color: '#7EB26D' },
......
......@@ -10,8 +10,7 @@ export const useSpinner = <T>(spinnerLabel: string, fn: FnToSpin<T>, killProcess
await fn(options);
spinner.succeed();
} catch (e) {
spinner.fail();
console.log(e);
spinner.fail(e);
if (killProcess) {
process.exit(1);
}
......
......@@ -34,14 +34,17 @@ module.exports = function (grunt) {
]);
grunt.registerTask('no-only-tests', function () {
var files = grunt.file.expand('public/**/*_specs\.ts', 'public/**/*_specs\.js');
var files = grunt.file.expand(
'public/**/*@(_specs|\.test)\.@(ts|js|tsx|jsx)',
'packages/grafana-ui/**/*@(_specs|\.test)\.@(ts|js|tsx|jsx)'
);
files.forEach(function (spec) {
var rows = grunt.file.read(spec).split('\n');
rows.forEach(function (row) {
if (row.indexOf('.only(') > 0) {
grunt.log.errorlns(row);
grunt.fail.warn('found only statement in test: ' + spec)
grunt.fail.warn('found only statement in test: ' + spec);
}
});
});
......
# Frontend Style Guide
Generally we follow the Airbnb [React Style Guide](https://github.com/airbnb/javascript/tree/master/react).
Generally we follow the Airbnb [React Style Guide](https://github.com/airbnb/javascript/tree/master/react).
## Table of Contents
1. [Basic Rules](#basic-rules)
1. [File & Component Organization](#Organization)
1. [Naming](#naming)
1. [Declaration](#declaration)
1. [Props](#props)
1. [Refs](#refs)
1. [Methods](#methods)
1. [Ordering](#ordering)
1. [Basic Rules](#basic-rules)
1. [File & Component Organization](#Organization)
1. [Naming](#naming)
1. [Declaration](#declaration)
1. [Props](#props)
1. [Refs](#refs)
1. [Methods](#methods)
1. [Ordering](#ordering)
## Basic rules
* Try to keep files small and focused and break large components up into sub components.
- Try to keep files small and focused and break large components up into sub components.
## Organization
* Components and types that needs to be used by external plugins needs to go into @grafana/ui
* Components should get their own folder under features/xxx/components
* Sub components can live in that component folders, so small component do not need their own folder
* Place test next to their component file (same dir)
* Component sass should live in the same folder as component code
* State logic & domain models should live in features/xxx/state
* Containers (pages) can live in feature root features/xxx
* up for debate?
- Components and types that needs to be used by external plugins needs to go into @grafana/ui
- Components should get their own folder under features/xxx/components
- Sub components can live in that component folders, so small component do not need their own folder
- Place test next to their component file (same dir)
- Component sass should live in the same folder as component code
- State logic & domain models should live in features/xxx/state
- Containers (pages) can live in feature root features/xxx
- up for debate?
## Props
* Name callback props & handlers with a "on" prefix.
- Name callback props & handlers with a "on" prefix.
```tsx
// good
......@@ -56,5 +56,32 @@ render() {
}
```
- React Component definitions
```jsx
// good
export class YourClass extends PureComponent<{},{}> { ... }
// bad
export class YourClass extends PureComponent { ... }
```
- React Component constructor
```typescript
// good
constructor(props:Props) {...}
// bad
constructor(props) {...}
```
- React Component defaultProps
```typescript
// good
static defaultProps: Partial<Props> = { ... }
// bad
static defaultProps = { ... }
```
# Pull Request Review Checklist
## High level checks
- [ ] The pull request adds value and the impact of the change is in line with [Frontend Style Guide](https://github.com/grafana/grafana/blob/master/style_guides/frontend.md).
- [ ] The pull request works the way it says it should do.
- [ ] The pull request does not increase the Angular code base.
> We are in the process of migrating to React so any increment of Angular code is generally discouraged from. (there are a few exceptions)
- [ ] The pull request closes one issue if possible and does not fix unrelated issues within the same pull request.
- [ ] The pull request contains necessary tests.
## Low level checks
- [ ] The pull request contains a title that explains the PR.
- [ ] The pull request contains necessary link(s) to issue(s).
- [ ] The pull request contains commits with commit messages that are small and understandable.
- [ ] The pull request does not contain magic strings or numbers that could be replaced with an `Enum` or `const` instead.
### Bug specific checks
- [ ] The pull request contains only one commit if possible.
- [ ] The pull request contains `closes: #Issue` or `fixes: #Issue` in pull request description.
## Frontend specific checks
- [ ] The pull request does not increase the number of `implicit any` errors.
- [ ] The pull request does not contain uses of `any` or `{}` without comments describing why.
- [ ] The pull request does not contain large React component that could easily be split into several smaller components.
- [ ] The pull request does not contain back end calls directly from components, use actions and Redux instead.
### Redux specific checks (skip if pull request does not contain Redux changes)
- [ ] The pull request does not contain code that mutate state in reducers or thunks.
- [ ] The pull request uses helpers `actionCreatorFactory` and `reducerFactory` instead of traditional `switch statement` reducers in Redux. ([Redux framework](https://github.com/grafana/grafana/blob/master/style_guides/redux.md))
- [ ] The pull request uses `reducerTester` to test reducers.([Redux framework](https://github.com/grafana/grafana/blob/master/style_guides/redux.md))
- [ ] The pull request does not contain code that access reducers state slice directly, instead the code uses state selectors to access state.
# Redux framework
To reduce the amount of boilerplate code used to create a strongly typed redux solution with actions, action creators, reducers and tests we've introduced a small framework around Redux.
`+` Much less boilerplate code
`-` Non Redux standard api
## New core functionality
### actionCreatorFactory
Used to create an action creator with the following signature
```typescript
{ type: string , (payload: T): {type: string; payload: T;} }
```
where the `type` string will be ensured to be unique and `T` is the type supplied to the factory.
#### Example
```typescript
export const someAction = actionCreatorFactory<string>('SOME_ACTION').create();
// later when dispatched
someAction('this rocks!');
```
```typescript
// best practices, always use an interface as type
interface SomeAction {
data: string;
}
export const someAction = actionCreatorFactory<SomeAction>('SOME_ACTION').create();
// later when dispatched
someAction({ data: 'best practices' });
```
```typescript
// declaring an action creator with a type string that has already been defined will throw
export const someAction = actionCreatorFactory<string>('SOME_ACTION').create();
export const theAction = actionCreatorFactory<string>('SOME_ACTION').create(); // will throw
```
### noPayloadActionCreatorFactory
Used when you don't need to supply a payload for your action. Will create an action creator with the following signature
```typescript
{ type: string , (): {type: string; payload: undefined;} }
```
where the `type` string will be ensured to be unique.
#### Example
```typescript
export const noPayloadAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create();
// later when dispatched
noPayloadAction();
```
```typescript
// declaring an action creator with a type string that has already been defined will throw
export const noPayloadAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create();
export const noAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create(); // will throw
```
### reducerFactory
Fluent API used to create a reducer. (same as implementing the standard switch statement in Redux)
#### Example
```typescript
interface ExampleReducerState {
data: string[];
}
const intialState: ExampleReducerState = { data: [] };
export const someAction = actionCreatorFactory<string>('SOME_ACTION').create();
export const otherAction = actionCreatorFactory<string[]>('Other_ACTION').create();
export const exampleReducer = reducerFactory<ExampleReducerState>(intialState)
// addMapper is the function that ties an action creator to a state change
.addMapper({
// action creator to filter out which mapper to use
filter: someAction,
// mapper function where the state change occurs
mapper: (state, action) => ({ ...state, data: state.data.concat(action.payload) }),
})
// a developer can just chain addMapper functions until reducer is done
.addMapper({
filter: otherAction,
mapper: (state, action) => ({ ...state, data: action.payload }),
})
.create(); // this will return the reducer
```
#### Typing limitations
There is a challenge left with the mapper function that I can not solve with TypeScript. The signature of a mapper is
```typescript
<State, Payload>(state: State, action: ActionOf<Payload>) => State;
```
If you would to return an object that is not of the state type like the following mapper
```typescript
mapper: (state, action) => ({ nonExistingProperty: ''}),
```
Then you would receive the following compile error
```shell
[ts] Property 'data' is missing in type '{ nonExistingProperty: string; }' but required in type 'ExampleReducerState'. [2741]
```
But if you return an object that is spreading state and add a non existing property type like the following mapper
```typescript
mapper: (state, action) => ({ ...state, nonExistingProperty: ''}),
```
Then you would not receive any compile error.
If you want to make sure that never happens you can just supply the State type to the mapper callback like the following mapper:
```typescript
mapper: (state, action): ExampleReducerState => ({ ...state, nonExistingProperty: 'kalle' }),
```
Then you would receive the following compile error
```shell
[ts]
Type '{ nonExistingProperty: string; data: string[]; }' is not assignable to type 'ExampleReducerState'.
Object literal may only specify known properties, and 'nonExistingProperty' does not exist in type 'ExampleReducerState'. [2322]
```
## New test functionality
### reducerTester
Fluent API that simplifies the testing of reducers
#### Example
```typescript
reducerTester()
.givenReducer(someReducer, initialState)
.whenActionIsDispatched(someAction('reducer tests'))
.thenStateShouldEqual({ ...initialState, data: 'reducer tests' });
```
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment