cosmos-explorer/CODING_GUIDELINES.md

195 lines
7.2 KiB
Markdown

# Coding Guidelines and Recommendations
Cosmos Explorer has been under constant development for over 5 years. As a result, there are many different patterns and practices in the codebase. This document serves as a guide to how we write code and helps avoid propagating practices which are no longer preferred. Each requirement in this document is labeled and color-coded to show the relative importance. In order from highest to lowest importance:
✅ DO this. If you feel you need an exception, engage with the project owners _prior_ to implementation.
⛔️ DO NOT do this. If you feel you need an exception, engage with the project owners _prior_ to implementation.
☑️ YOU SHOULD strongly consider this but it is not a requirement. If not following this advice, please comment code with why and proactively begin a discussion as part of the PR process.
⚠️ YOU SHOULD NOT strongly consider not doing this. If not following this advice, please comment code with why and proactively begin a discussion as part of the PR process.
💭 YOU MAY consider this advice if appropriate to your situation. Other team members may comment on this as part of PR review, but there is no need to be proactive.
## Development Environment
☑️ YOU SHOULD
- Use VSCode and install the following extensions. This setup will catch most linting/formatting/type errors as you develop:
- [Prettier](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode)
- [ESLint](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint)
💭 YOU MAY
- Use the [GitHub CLI](https://cli.github.com/). It has helpful workflows for submitting PRs as well as for checking out other team member's PRs.
- Use Windows, Linux (including WSL), or OSX. We have team members developing on all three environments.
✅ DO
- Maintain cross-platform compatibility when modifying any engineering or build systems
## Code Formatting
✅ DO
- Use [Prettier](https://prettier.io/) to format your code
- This will occur automatically if using the recommended editor setup
- `npm run format` will also format code
## Linting
✅ DO
- Use [ESLint](https://eslint.org/) to check for code errors.
- This will occur automatically if using the recommended editor setup
- `npm run lint` will also check for linting errors
💭 YOU MAY
- Consider adding new lint rules.
- If you find yourself performing "nits" as part of PR review, consider adding a lint rule that will automatically catch the error in the future
⚠️ YOU SHOULD NOT
- Disable lint rules
- Lint rules exist as guidance and to catch common mistakes
- You will find places we disable specific lint rules however it should be exceptional.
- If a rule does need to be disabled, prefer disabling a specific line instead of the entire file.
⛔️ DO NOT
- Add [TSLint](https://palantir.github.io/tslint/) rules
- TSLint has been deprecated and is on track to be removed
- Always prefer ESLint rules
## UI Components
☑️ YOU SHOULD
- Write new components using [React](https://reactjs.org/). We are actively migrating Cosmos Explorer off of [Knockout](https://knockoutjs.com/).
- Use [Fluent](https://developer.microsoft.com/en-us/fluentui#/) components.
- Fluent components are designed to be highly accessible and composable
- Using Fluent allows us to build upon the work of the Fluent team and leads to a lower total cost of ownership for UI code
### React
☑️ YOU SHOULD
- Use pure functional components when no state is required
💭 YOU MAY
- Use functional (hooks) or class components
- The project contains examples of both
- Neither is strongly preferred at this time
⛔️ DO NOT
- Use inheritance for sharing component behavior.
- React documentation covers this topic in detail https://reactjs.org/docs/composition-vs-inheritance.html
- Suffix your file or component name with "Component"
- Even though the code has examples of it, we are ending the practice.
## Libraries
⚠️ YOU SHOULD NOT
- Add new libraries to package.json.
- Adding libraries may bring in code that explodes the bundled size or attempts to run NodeJS code in the browser
- Consult with project owners for help with library selection if one is needed
⛔️ DO NOT
- Use underscore.js
- Much of this library is now native to JS and will be automatically transpiled
- Use jQuery
- Much of this library is not native to the DOM.
- We are planning to remove it
## Testing
⛔️ DO NOT
- Decrease test coverage
- Unit/Functional test coverage is checked as part of the CI process
### Unit Tests
✅ DO
- Write unit tests for non-UI and utility code.
- Write your tests using [Jest](https://jestjs.io/)
☑️ YOU SHOULD
- Abstract non-UI and utility code so it can run either the NodeJS or Browser environment
### Functional(Component) Tests
✅ DO
- Write tests for UI components
- Write your tests using [Jest](https://jestjs.io/)
- Use either Enzyme or React Testing Library to perform component tests.
### Mocking
✅ DO
- Use Jest's built-in mocking helpers
☑️ YOU SHOULD
- Write code that does not require mocking
- Build components that do not require mocking extremely large or difficult to mock objects (like Explorer.ts). Pass _only_ what you need.
⛔️ DO NOT
- Use sinon.js for mocking
- Sinon has been deprecated and planned for removal
### End to End Tests
✅ DO
- Use [Puppeteer](https://developers.google.com/web/tools/puppeteer) and [Jest](https://jestjs.io/)
- Write or modify an existing E2E test that covers the primary use case of any major feature.
- Use caution. Do not try to cover every case. End to End tests can be slow and brittle.
☑️ YOU SHOULD
- Write tests that use accessible attributes to perform actions. Role, Title, Label, etc
- More information https://testing-library.com/docs/queries/about#priority
⚠️ YOU SHOULD NOT
- Add test specfic `data-*` attributes to dom elements
- This is a common current practice, but one we would like to avoid in the future
- End to end tests need to use semantic HTML and accesible attributes to be truely end to end
- No user or screen reader actually navigates an app using `data-*` attributes
- Add arbitrary time delays to wait for page to render or element to be ready.
- All the time delays add up and slow down testing.
- Prefer using the framework's "wait for..." functionality.
### Migrating Knockout to React
✅ DO
- Consult other team members before beginning migration work. There is a significant amount of flux in patterns we are using and it is important we do not propagate incorrect patterns.
- Start by converting HTML to JSX: https://magic.reactjs.net/htmltojsx.htm. Add functionality as a second step.
☑️ YOU SHOULD
- Write React components that require no dependency on Knockout or observables to trigger rendering.
## Browser Support
✅ DO
- Support all [browsers supported by the Azure Portal](https://docs.microsoft.com/en-us/azure/azure-portal/azure-portal-supported-browsers-devices)
- Support IE11
- In practice, this should not need to be considered as part of a normal development workflow
- Polyfills and transpilation are already provided by our engineering systems.
- This requirement will be removed on March 30th, 2021 when Azure drops IE11 support.