From c5a3dfe1a67184fe9fc215907030506db404a3eb Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 9 Feb 2018 10:53:07 +0000 Subject: [PATCH 1/2] Improve docs about allowing some side effects on the constructor --- doc/development/fe_guide/style_guide_js.md | 33 ++++++++++++++++++++-- 1 file 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 02773162801..2e2d58b6d66 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 it's just an example. If the all 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 inited 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' } ]; From ee8ac683f704849d5d57a5f943a97b4ffbc2aeb4 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 9 Feb 2018 12:01:05 +0000 Subject: [PATCH 2/2] Fix english in style_guide_js.md --- doc/development/fe_guide/style_guide_js.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 2e2d58b6d66..26eda80685e 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -210,7 +210,7 @@ Do not use them anymore and feel free to remove them when refactoring legacy cod 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 it's just an example. If the all purpose of the class is to add an event listener and handle the callback a function will be more suitable.) +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 @@ -236,7 +236,7 @@ export class Foo { } ``` -On the other hand, if a class only needs to extend a third party/add event listeners in some specific cases, they should be inited oustside of the constructor. +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 most likely cause side effects, it will be mutating the array being iterated. Prefer using `.map`,