summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2018-02-09 17:50:41 +0000
committerPhil Hughes <me@iamphill.com>2018-02-09 17:50:41 +0000
commit721fab661de4a01c2d73e88bdd000dfe2e094ced (patch)
tree485aa7827057943b2dcf5ad7c3121af81676fab2
parentb8ee24e65f351bf41f77db253329502c92241666 (diff)
parentee8ac683f704849d5d57a5f943a97b4ffbc2aeb4 (diff)
downloadgitlab-ce-721fab661de4a01c2d73e88bdd000dfe2e094ced.tar.gz
Merge branch 'doc-improve-side-effects-block' into 'master'
Improve docs about allowing some side effects on the constructor See merge request gitlab-org/gitlab-ce!17027
-rw-r--r--doc/development/fe_guide/style_guide_js.md33
1 files changed, 31 insertions, 2 deletions
diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md
index cd26baa4243..917d28b48ee 100644
--- a/doc/development/fe_guide/style_guide_js.md
+++ b/doc/development/fe_guide/style_guide_js.md
@@ -207,10 +207,39 @@ Do not use them anymore and feel free to remove them when refactoring legacy cod
var c = pureFunction(values.foo);
```
-1. Avoid constructors with side-effects
+1. Avoid constructors with side-effects.
+Although we aim for code without side-effects we need some side-effects for our code to run.
+
+If the class won't do anything if we only instantiate it, it's ok to add side effects into the constructor (_Note:_ The following is just an example. If the only purpose of the class is to add an event listener and handle the callback a function will be more suitable.)
+
+```javascript
+// Bad
+export class Foo {
+ constructor() {
+ this.init();
+ }
+ init() {
+ document.addEventListener('click', this.handleCallback)
+ },
+ handleCallback() {
+
+ }
+}
+
+// Good
+export class Foo {
+ constructor() {
+ document.addEventListener()
+ }
+ handleCallback() {
+ }
+}
+```
+
+On the other hand, if a class only needs to extend a third party/add event listeners in some specific cases, they should be initialized oustside of the constructor.
1. Prefer `.map`, `.reduce` or `.filter` over `.forEach`
-A forEach will cause side effects, it will be mutating the array being iterated. Prefer using `.map`,
+A forEach will most likely cause side effects, it will be mutating the array being iterated. Prefer using `.map`,
`.reduce` or `.filter`
```javascript
const users = [ { name: 'Foo' }, { name: 'Bar' } ];