In the list below, I'm referring to a preferred ways of doing stuff. I use the (TypeScript) rules from our package https://www.npmjs.com/package/@nlesc/tslint-config-react-app to determine what's preferred. In some other cases, I specifically wrote 'I prefer' to show it's my personal preference.
in src/App.js
[ ] mix of tabs and spaces in package.json (preferred: 4 spaces). there's automatic tools that can helpyou with this
[ ] the preferred way of writing {activeNode: activeNode} is {activeNode}
[ ] line 25: you're using function() {}. The preferred way (in TS) is to use lambda functions, e.g. (prevState, props) => {// do stuff}
[ ] You're using var. The scoping rules of var are counterintuitive, therefore there's now const and let. There is no longer any case where var is preferred.
[ ] split loops over multiple lines, with the opening squiggly brace on the first line
[ ] line 42 inDegreeValue shouldn't that be inDecreeValue?
[ ] line 50 const activeNode = this.state.activeNode; I prefer the new way of writing this: const { activeNode } = this.state;
[ ] line 53 return not aligned
in src/Network.js
[ ] tabs v spaces, use 4 spaces
[ ] line 2. you wrote import {Sigma, LoadJSON, RandomizeNodePositions, RelativeSize, EdgeShapes, Filter} from 'react-sigma'. I prefer to split them over multiple lines, with one import per line: import { Sigma } from 'react-sigma'; import { LoadJSON } from 'react-sigma'; etc. Easier for development, easier for diffs, easier for reading AFAIC
[ ] line 31 var _onGraphLoaded = function() {} becomes const _onGraphLoaded = () => {}. I personally don't use the _ anymore. This was once introduced to denote a private/local function, but with the use of const that is already implied (blcokscoped).
[ ] line 38. This construct for (i = 0; i < len_n; i++) { should usually be replaced by a forEach loop, so you'd get nodes.forEach(f) (where f is something like const f = (node) => {//the function body};
[ ] line 56: setting the color is maybe more suitably set with CSS. I'm assuming there's a CSS class for sigma edges somewhere, you'd then include something like theCSSClass {color: #999;} in the relevant CSS file. I believe webpack is smart enough to package the CSS file with the JS|TS file if you do an import of the CSS, like so import './Network.css'.
[ ] line 61-65: you could do const {filterInDegree, filterSubject, minYearValue, maxYearValue, sizeAttributeValue = this.props.filterState; instead. Myabe split over 2 lines for radability.
[ ] line 68: I prefer to always use parentheses and squiggly braces when using lambdas, and to always split the lamda over multiple lines, even when the body is one line.
in AttributesPane.js
[ ] line 20-23: I'd prefer const {title, id date, articles_s, abstract} = this.props.activeNode; because it avoids repeating this.props.activeNode 6 times in the render method
in src/FilterPane.js
[ ] line 57: I'd add squiggly braces, a return, and I'd break the body over multiple lines
That's pretty much it. It looks like a long list but don;t be discouraged, I was in full pedantic mode, plus at least half of these items go away once you adopt https://www.npmjs.com/package/@nlesc/react-scripts (it comes with a linter and NLeSC TypeScript rules). Good luck and let me know if you run into problems.
(engage pedantic mode)
code state: https://github.com/NLeSC/case-law-app/tree/e12223ba104536b84e7b7157e6c54d6005b29fa5
In the list below, I'm referring to a preferred ways of doing stuff. I use the (TypeScript) rules from our package https://www.npmjs.com/package/@nlesc/tslint-config-react-app to determine what's preferred. In some other cases, I specifically wrote 'I prefer' to show it's my personal preference.
{activeNode: activeNode}is{activeNode}import Network from './components/Network';(in TypeScript) I prefer to useimport Network from './components';(note that you need an extra filecomponents/index.tsfor this to work properly, see https://github.com/NLeSC-Storyteller/query-builder-client/tree/d8d65c20e6008b0393b15e083fa2580582b18111/src/components for an example)function() {}. The preferred way (in TS) is to use lambda functions, e.g. (prevState, props) => {// do stuff}var. The scoping rules ofvarare counterintuitive, therefore there's nowconstandlet. There is no longer any case wherevaris preferred.inDegreeValueshouldn't that beinDecreeValue?const activeNode = this.state.activeNode;I prefer the new way of writing this:const { activeNode } = this.state;import {Sigma, LoadJSON, RandomizeNodePositions, RelativeSize, EdgeShapes, Filter} from 'react-sigma'. I prefer to split them over multiple lines, with one import per line:import { Sigma } from 'react-sigma'; import { LoadJSON } from 'react-sigma';etc. Easier for development, easier for diffs, easier for reading AFAICvar _onGraphLoaded = function() {}becomesconst _onGraphLoaded = () => {}. I personally don't use the_anymore. This was once introduced to denote a private/local function, but with the use ofconstthat is already implied (blcokscoped).for (i = 0; i < len_n; i++) {should usually be replaced by a forEach loop, so you'd getnodes.forEach(f)(where f is something likeconst f = (node) => {//the function body};theCSSClass {color: #999;}in the relevant CSS file. I believe webpack is smart enough to package the CSS file with the JS|TS file if you do an import of the CSS, like soimport './Network.css'.const {filterInDegree, filterSubject, minYearValue, maxYearValue, sizeAttributeValue = this.props.filterState;instead. Myabe split over 2 lines for radability.const {title, id date, articles_s, abstract} = this.props.activeNode;because it avoids repeatingthis.props.activeNode6 times in therendermethod in src/FilterPane.jsreturn, and I'd break the body over multiple linesThat's pretty much it. It looks like a long list but don;t be discouraged, I was in full pedantic mode, plus at least half of these items go away once you adopt https://www.npmjs.com/package/@nlesc/react-scripts (it comes with a linter and NLeSC TypeScript rules). Good luck and let me know if you run into problems.
-Jurriaan