|
1 | 1 | # Coding Guidelines
|
2 | 2 |
|
3 |
| -New code files should adhere to the coding guidelines listed in this document. |
4 |
| -Existing code may or may not conform to these guidelines and contributors are |
5 |
| -encouraged to refactor code to follow the guidelines when it is relevant to do |
6 |
| -so. |
| 3 | +This page is deprecated. |
7 | 4 |
|
8 |
| -The purpose of the guidelines is to ensure code remains maintainable and easy to |
9 |
| -read as the code base grows. |
10 |
| - |
11 |
| -## Adding new rules |
12 |
| - |
13 |
| -Add new rules at the bottom of the list, incrementing the rule number by 1. Do |
14 |
| -not re-order rules so that they keep a stable number. |
15 |
| - |
16 |
| -## Guidelines |
17 |
| - |
18 |
| -Rules: |
19 |
| - |
20 |
| -- [R001: Use utility functions to create CSS class names](#r001-use-utility-functions-to-create-css-class-names) |
21 |
| -- [R002: Avoid `condition ? true : false`](#r002-avoid-condition--true--false) |
22 |
| -- [R003: Avoid nested `if/else` blocks](#r003-avoid-nested-ifelse-blocks) |
23 |
| -- [R004: Include a default export when appropriate](#r004-include-a-default-export-when-appropriate) |
24 |
| -- [R005: File naming conventions](#r005-file-naming-conventions) |
25 |
| -- [R006: Use existing Bootstrap classes or import from CSS modules](#r006-use-existing-bootstrap-classes-or-import-from-css-modules) |
26 |
| -- [R007: Do not perform async actions in React hooks](#r007-do-not-perform-async-actions-in-react-hooks) |
27 |
| -- [R008: Interactive handlers can only be used on interactive HTML tags](#r008-interactive-handlers-can-only-be-used-on-interactive-html-tags) |
28 |
| - |
29 |
| -### R001: Use utility functions to create CSS class names |
30 |
| - |
31 |
| -**✅ DO** |
32 |
| - |
33 |
| -```ts |
34 |
| -import cx from "classnames"; |
35 |
| - |
36 |
| -const className = cx( |
37 |
| - "rounded", |
38 |
| - disabled && "btn-disabled", |
39 |
| - color && `btn-${color}` |
40 |
| -); |
41 |
| -``` |
42 |
| - |
43 |
| -**❌ DON'T** |
44 |
| - |
45 |
| -```ts |
46 |
| -const className = "rounded" + (disabled ? " disabled" : ""); |
47 |
| -``` |
48 |
| - |
49 |
| -**💡 Rationale** |
50 |
| - |
51 |
| -Constructing CSS class names by hand leads to frequent mistakes, e.g. |
52 |
| -having `undefined` or `null` as one of the CSS classes of an HTML element. |
53 |
| - |
54 |
| -**💭 Tip** |
55 |
| - |
56 |
| -We agreed on using `cx` in every case, except when there is only |
57 |
| -_one_ class name as a string. |
58 |
| - |
59 |
| -```tsx |
60 |
| -const padding = someCondition ? "p-2" : "p-3"; |
61 |
| - |
62 |
| -<MyComponent className="p-2" /> |
63 |
| -<MyComponent className={cx("p-2", "text-danger")} /> |
64 |
| -<MyComponent className={cx(padding)} /> |
65 |
| - |
66 |
| -``` |
67 |
| - |
68 |
| -### R002: Avoid `condition ? true : false` |
69 |
| - |
70 |
| -**✅ DO** |
71 |
| - |
72 |
| -```ts |
73 |
| -const isActive = conditionA && x > y && conditionC; |
74 |
| -``` |
75 |
| - |
76 |
| -**❌ DON'T** |
77 |
| - |
78 |
| -```ts |
79 |
| -const isActive = conditionA && x > y && conditionC ? true : false; |
80 |
| -``` |
81 |
| - |
82 |
| -**💡 Rationale** |
83 |
| - |
84 |
| -The `? true : false` construct is unnecessary and may surprise the reader, |
85 |
| -leading to slower code reading. |
86 |
| - |
87 |
| -**💭 Tip** |
88 |
| - |
89 |
| -Use double boolean negation to ensure the variable is of type `boolean`. |
90 |
| - |
91 |
| -```ts |
92 |
| -const enabled = !!objMaybeUndefined?.field.length; |
93 |
| -``` |
94 |
| - |
95 |
| -### R003: Avoid nested `if/else` blocks |
96 |
| - |
97 |
| -**✅ DO** |
98 |
| - |
99 |
| -```ts |
100 |
| -function getStatus(input: Input) { |
101 |
| - if (condA && condB && condC) { |
102 |
| - return "success"; |
103 |
| - } |
104 |
| - if (condA && condB) { |
105 |
| - return "fairly good"; |
106 |
| - } |
107 |
| - if (condA) { |
108 |
| - return "warning"; |
109 |
| - } |
110 |
| - if (condB && condC) { |
111 |
| - return "critical"; |
112 |
| - } |
113 |
| - return "not quite there"; |
114 |
| -} |
115 |
| -``` |
116 |
| - |
117 |
| -**❌ DON'T** |
118 |
| - |
119 |
| -```ts |
120 |
| -function getStatus(input: Input) { |
121 |
| - if (condA) { |
122 |
| - if (condB) { |
123 |
| - if (condC) { |
124 |
| - return "success"; |
125 |
| - } else { |
126 |
| - return "fairly good"; |
127 |
| - } |
128 |
| - } else { |
129 |
| - return "warning"; |
130 |
| - } |
131 |
| - } else if (condB && condC) { |
132 |
| - return "critical"; |
133 |
| - } else { |
134 |
| - return "not quite there"; |
135 |
| - } |
136 |
| -} |
137 |
| -``` |
138 |
| - |
139 |
| -**💡 Rationale** |
140 |
| - |
141 |
| -Nested `if/else` block are hard to read and follow, especially when |
142 |
| -they span more than one screen vertically. |
143 |
| - |
144 |
| -If possible, use early returns to flatten the case enumeration. This means in |
145 |
| -some cases, separating this logic into a utility function is recommended. |
146 |
| - |
147 |
| -When the logic is short, cascading ternary operators can be used, e.g. |
148 |
| - |
149 |
| -```ts |
150 |
| -const status = |
151 |
| - response === "hello" |
152 |
| - ? "greeting" |
153 |
| - : response === "bye" |
154 |
| - ? "leaving" |
155 |
| - : response.startsWith("says:") |
156 |
| - ? "talking" |
157 |
| - : "idle"; |
158 |
| -``` |
159 |
| - |
160 |
| -### R004: Include a default export when appropriate |
161 |
| - |
162 |
| -Include a `default` export when a main component is defined in a source file |
163 |
| -(this should be the usual case). |
164 |
| - |
165 |
| -**✅ DO** |
166 |
| - |
167 |
| -```tsx |
168 |
| -export default function MyComponent() { |
169 |
| - return ( |
170 |
| - <div> |
171 |
| - <h2>My Component</h2> |
172 |
| - <p>Hello, World!</p> |
173 |
| - </div> |
174 |
| - ); |
175 |
| -} |
176 |
| -``` |
177 |
| - |
178 |
| -**💡 Rationale** |
179 |
| - |
180 |
| -The main component being exported as a `default` export is a common convention |
181 |
| -with `React` projects. |
182 |
| - |
183 |
| -The `default` export also shows which component is a the top level and which |
184 |
| -ones are sub-components. |
185 |
| - |
186 |
| -### R005: File naming conventions |
187 |
| - |
188 |
| -**React Components** |
189 |
| - |
190 |
| -Use the component name as a file name. The file name as well as the component |
191 |
| -name should use CamelCase. |
192 |
| - |
193 |
| -Example: `MyComponent.tsx` |
194 |
| - |
195 |
| -**Type Definitions** |
196 |
| - |
197 |
| -Type definition file names start with a lowercase letter and end with |
198 |
| -`.types.ts`. |
199 |
| - |
200 |
| -Example: `workflows.types.ts` |
201 |
| - |
202 |
| -**Hook Definitions** |
203 |
| - |
204 |
| -Reusable hooks should be defined in their own file, be a default export, |
205 |
| -use the hook name as a file name and have the extension `.hook.ts`. |
206 |
| - |
207 |
| -Example: `useRenku.hook.ts` |
208 |
| - |
209 |
| -**Utility Functions** |
210 |
| - |
211 |
| -Utility file names start with a lowercase letter and end with `.utils.ts`. |
212 |
| - |
213 |
| -Example: `sessions.utils.ts` |
214 |
| - |
215 |
| -**API Definitions** |
216 |
| - |
217 |
| -Files defining RTK Query endpoints start with a lowercase letter and end with |
218 |
| -`.api.ts`. |
219 |
| - |
220 |
| -Example: `projects.api.ts` |
221 |
| - |
222 |
| -**Slice Definitions** |
223 |
| - |
224 |
| -Files defining RTK slices start with a lowercase letter and end with |
225 |
| -`.slice.ts`. |
226 |
| - |
227 |
| -Example: `user.slice.ts` |
228 |
| - |
229 |
| -**Test Definitions** |
230 |
| - |
231 |
| -Test file names start with a lowercase letter and end with `.test.ts` for unit |
232 |
| -tests or with `.spec.ts` for Cypress tests. |
233 |
| - |
234 |
| -Example: `login.test.ts` or `datasets.spec.ts` |
235 |
| - |
236 |
| -### R006: Use existing Bootstrap classes or import from CSS modules |
237 |
| - |
238 |
| -**✅ DO** |
239 |
| - |
240 |
| -```tsx |
241 |
| -<Card className={cx("m-3", "py-2")} /> |
242 |
| -``` |
243 |
| - |
244 |
| -```tsx |
245 |
| -import styles from "SpecialCard.module.scss"; |
246 |
| - |
247 |
| -<Card className={cx(styles.projectCard)} />; |
248 |
| -``` |
249 |
| - |
250 |
| -**❌ DON'T** |
251 |
| - |
252 |
| -```tsx |
253 |
| -import "./SpecialCard.css"; |
254 |
| - |
255 |
| -<Card className="my-special-card-class" />; |
256 |
| -``` |
257 |
| - |
258 |
| -**💡 Rationale** |
259 |
| - |
260 |
| -We want to avoid an explosion of the number of CSS classes, as well as |
261 |
| -classes polluting the global namespace. |
262 |
| -Since everyone knows Bootstrap, it is a good idea to use it as much as |
263 |
| -possible. Should the need arise for very specific styling, we can always |
264 |
| -create a new class in a local (S)CSS module file and import it. |
265 |
| - |
266 |
| -**💭 Tip** |
267 |
| - |
268 |
| -We want to push for sticking to Bootstrap's utility classes as much as possible. |
269 |
| -This means we should discuss for deviations from a reference design when that |
270 |
| -only minimally impacts the interface. |
271 |
| - |
272 |
| -E.G. If a Figma design reference file shows a distance between components |
273 |
| -of 14.5px and `m-3` is 16px, we should use `m-3` instead. |
274 |
| - |
275 |
| -### R007: Do not perform async actions in React hooks |
276 |
| - |
277 |
| -Do not use `.then()` or `.unwrap()` inside React hooks. Instead, listen to the result of the async action and act on the result. |
278 |
| - |
279 |
| -**✅ DO** |
280 |
| - |
281 |
| -```tsx |
282 |
| -function MyComponent() { |
283 |
| - const [postRequest, result] = usePostRequest(); |
284 |
| - |
285 |
| - const onClick = useCallback(() => { |
286 |
| - postRequest(); |
287 |
| - }, [postRequest]); |
288 |
| - |
289 |
| - useEffect(() => { |
290 |
| - if (result.isSuccess) { |
291 |
| - // Do something... |
292 |
| - } |
293 |
| - }, [result.isSuccess]); |
294 |
| - |
295 |
| - return ( |
296 |
| - <div> |
297 |
| - <button onClick={onClick}>Some action</button> |
298 |
| - </div> |
299 |
| - ); |
300 |
| -} |
301 |
| -``` |
302 |
| - |
303 |
| -**❌ DON'T** |
304 |
| - |
305 |
| -```tsx |
306 |
| -function MyComponent() { |
307 |
| - const [postRequest, result] = usePostRequest(); |
308 |
| - |
309 |
| - const onClick = useCallback(() => { |
310 |
| - postRequest() |
311 |
| - .unwrap() |
312 |
| - .then(() => { |
313 |
| - // Do something... |
314 |
| - }); |
315 |
| - }, [postRequest]); |
316 |
| - |
317 |
| - return ( |
318 |
| - <div> |
319 |
| - <button onClick={onClick}>Some action</button> |
320 |
| - </div> |
321 |
| - ); |
322 |
| -} |
323 |
| -``` |
324 |
| - |
325 |
| -**💡 Rationale** |
326 |
| - |
327 |
| -Calling code after an async action may happen after a component has re-rendered or has been removed. |
328 |
| -In this case, if the corresponding code tries to access the component, it will cause an error. |
329 |
| - |
330 |
| -### R008: Interactive handlers can only be used on interactive HTML tags |
331 |
| - |
332 |
| -Do not add interactive handlers, e.g. `onClick`, to HTML tags which are not interactive. |
333 |
| - |
334 |
| -**✅ DO** |
335 |
| - |
336 |
| -```tsx |
337 |
| -function MyComponent() { |
338 |
| - return ( |
339 |
| - <ul> |
340 |
| - <li> |
341 |
| - <a href="..."> |
342 |
| - My Content |
343 |
| - </a> |
344 |
| - <li> |
345 |
| - </ul> |
346 |
| - ); |
347 |
| -} |
348 |
| -``` |
349 |
| - |
350 |
| -```tsx |
351 |
| -function MyComponent() { |
352 |
| - const onClick = ...Some action...; |
353 |
| - |
354 |
| - return ( |
355 |
| - <ul> |
356 |
| - <li> |
357 |
| - <button onClick={onClick}> |
358 |
| - My Content |
359 |
| - </button> |
360 |
| - <li> |
361 |
| - </ul> |
362 |
| - ); |
363 |
| -} |
364 |
| -``` |
365 |
| - |
366 |
| -**❌ DON'T** |
367 |
| - |
368 |
| -```tsx |
369 |
| -function MyComponent() { |
370 |
| - const onClick = ...Some action...; |
371 |
| - |
372 |
| - return ( |
373 |
| - <ul> |
374 |
| - <li onClick={onClick}> |
375 |
| - My Content |
376 |
| - <li> |
377 |
| - </ul> |
378 |
| - ); |
379 |
| -} |
380 |
| -``` |
381 |
| - |
382 |
| -**💡 Rationale** |
383 |
| - |
384 |
| -Browsers do not expect tags such as `<div>` or `<span>` to be interactive, which means they will |
385 |
| -not get focused with keyboard navigation. Adding interactive handlers to non-interactive tags will |
386 |
| -therefore hinder accessibility to users which do not have access to a mouse or touch screen. |
| 5 | +The guidelines have been moved [to our wiki](https://github.com/SwissDataScienceCenter/renku-ui/wiki/Develop#coding-rules-). |
0 commit comments