Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ESLint warnings (1) #1060

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Fix ESLint warnings (1) #1060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 ui/src/actions/gphl_workflow.js → ui/src/actions/gphlWorkflow.js
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ui/src/containers/GphlWorkflowParametersDialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Form from '@rjsf/core';
import {
showGphlWorkflowParametersDialog,
gphlWorkflowSubmitParameters,
} from '../actions/gphl_workflow';
} from '../actions/gphlWorkflow';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No ESLint warning here, it's just for consistency.


class GphlWorkflowParametersDialog extends React.Component {
constructor(props) {
Expand Down
7 changes: 5 additions & 2 deletions ui/src/reducers/beamline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable sonarjs/no-duplicate-string */
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonarjs/no-duplicate-string: I'm disabling the rule in a couple of files — the rule can be a bit noisy but helps spot duplicated identifiers and such, so I prefer to not disable it globally

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

import { STATE } from '../actions/beamline';
import { RUNNING, MOTOR_STATE } from '../constants';

Expand Down Expand Up @@ -141,7 +142,7 @@ export const INITIAL_STATE = {
energyScanElements: [],
};

export default (state = INITIAL_STATE, action) => {
function beamlineReducer(state = INITIAL_STATE, action = {}) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super, thanks

let data = {};

switch (action.type) {
Expand Down Expand Up @@ -415,4 +416,6 @@ export default (state = INITIAL_STATE, action) => {
default:
return state;
}
};
}

export default beamlineReducer;
9 changes: 5 additions & 4 deletions ui/src/reducers/contextMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable import/no-anonymous-default-export */
const initialState = {
const INITIAL_STATE = {
show: false,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constant case by convention

shape: { type: 'NONE' },
x: 0,
Expand All @@ -12,7 +11,7 @@ const initialState = {
},
};

export default (state = initialState, action) => {
function contextMenuReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SHOW_CONTEXT_MENU': {
return {
Expand All @@ -36,4 +35,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default contextMenuReducer;
8 changes: 5 additions & 3 deletions ui/src/reducers/general.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const initialState = {
const INITIAL_STATE = {
loading: false,
showErrorPanel: false,
errorMessage: '',
Expand All @@ -13,7 +13,7 @@ const initialState = {
applicationFetched: false,
};

export default (state = initialState, action) => {
function generalReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_LOADING': {
return {
Expand Down Expand Up @@ -60,4 +60,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default generalReducer;
8 changes: 5 additions & 3 deletions ui/src/reducers/gphl_workflow.js → ui/src/reducers/gphlWorkflow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const initialState = {
const INITIAL_STATE = {
workflows: [],
current: null,
};

export default (state = initialState, action) => {
function gphlWorkflowReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_WORKFLOWS': {
return { ...state, workflows: { ...action.workflows } };
Expand All @@ -25,4 +25,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default gphlWorkflowReducer;
2 changes: 1 addition & 1 deletion ui/src/reducers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import contextMenu from './contextMenu';
import remoteAccess from './remoteAccess';
import shapes from './shapes';
import workflow from './workflow';
import gphl_workflow from './gphl_workflow';
import gphl_workflow from './gphlWorkflow';
import taskResult from './taskResult';
import uiproperties from './uiproperties';

Expand Down
8 changes: 5 additions & 3 deletions ui/src/reducers/logger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const initialState = {
const INITIAL_STATE = {
logRecords: [],
activePage: 0,
};

export default (state = initialState, action) => {
function loggerReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'ADD_LOG_RECORD': {
return { ...state, logRecords: [...state.logRecords, action.data] };
Expand All @@ -17,4 +17,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default loggerReducer;
8 changes: 5 additions & 3 deletions ui/src/reducers/login.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const initialState = {
const INITIAL_STATE = {
loggedIn: null, // null means loggedIn state is not known yet
showProposalsForm: false,
selectedProposal: '',
Expand All @@ -9,7 +9,7 @@ const initialState = {
},
};

export default (state = initialState, action) => {
function loginReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_LOGIN_INFO': {
if (action.loginInfo.user.username !== '') {
Expand Down Expand Up @@ -56,4 +56,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default loginReducer;
16 changes: 9 additions & 7 deletions ui/src/reducers/queue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import update from 'immutability-helper';
import { QUEUE_STOPPED, CLICK_CENTRING } from '../constants';
import { clearAllLastUsedParameters } from '../components/Tasks/fields';

const initialState = {
const INITIAL_STATE = {
queue: [],
current: { sampleID: null, running: false },
queueStatus: QUEUE_STOPPED,
Expand All @@ -14,21 +14,21 @@ const initialState = {
groupFolder: '',
};

export default (state = initialState, action) => {
function queueReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_QUEUE': {
return { ...state, queue: action.sampleOrder };
}
case 'CLEAR_QUEUE': {
return {
...state,
queue: initialState.queue,
current: initialState.current,
queue: INITIAL_STATE.queue,
current: INITIAL_STATE.current,
};
}
case 'ADD_SAMPLES_TO_QUEUE': {
const sampleIDList = action.samplesData.map((s) => s.sampleID);
return { ...state, queue: state.queue.concat(sampleIDList) };
return { ...state, queue: [...state.queue, ...sampleIDList] };
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer-spread: foo.concat(bar) => [...foo, ...bar] – this is just a stylistic rule, useful to ensure consistency in the codebase

}
case 'SET_QUEUE_STATUS':
return {
Expand Down Expand Up @@ -103,7 +103,7 @@ export default (state = initialState, action) => {
return { ...state, [action.settingName]: action.value };
}
case 'CLEAR_ALL': {
return { ...state, ...initialState, autoMountNext: state.autoMountNext };
return { ...state, ...INITIAL_STATE, autoMountNext: state.autoMountNext };
}
case 'QUEUE_STATE': {
return Object.assign({}, state, ...action.queueState);
Expand All @@ -128,4 +128,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default queueReducer;
14 changes: 8 additions & 6 deletions ui/src/reducers/queueGUI.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { omit } from 'lodash/object';

const initialState = {
const INITIAL_STATE = {
showRestoreDialog: false,
searchString: '',
displayData: {},
Expand All @@ -10,7 +10,7 @@ const initialState = {
showConfirmCollectDialog: false,
};

export default (state = initialState, action) => {
function queueGUIReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'redux-form/CHANGE': {
if (action.form === 'search-sample') {
Expand All @@ -26,7 +26,7 @@ export default (state = initialState, action) => {
const newNodes = [];

action.sampleOrder.forEach((sampleID) => {
if (action.sampleList.hasOwnProperty(sampleID)) {
if (sampleID in action.sampleList) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-prototype-builtins reports that foo.hasOwnProperty("bar") should be replaced with Object.prototype.hasOwnProperty.call(foo, "bar") to account for potential edge cases.

However, in the cases reported, hasOwnProperty can simply be replaced with the in operator, since we're only dealing with plain objects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice !

action.sampleList[sampleID].tasks.forEach((task) => {
if (!existingNodes.includes(task.queueID.toString())) {
newNodes.push(task.queueID.toString());
Expand Down Expand Up @@ -126,7 +126,7 @@ export default (state = initialState, action) => {
const newNodes = [];

sampleOrder.forEach((sampleID) => {
if (sampleList.hasOwnProperty(sampleID)) {
if (sampleID in sampleList) {
sampleList[sampleID].tasks.forEach((task) => {
if (!existingNodes.includes(task.queueID.toString())) {
newNodes.push(task.queueID.toString());
Expand All @@ -144,9 +144,11 @@ export default (state = initialState, action) => {
return { ...state, displayData };
}
case 'CLEAR_ALL': {
return initialState;
return INITIAL_STATE;
}
default:
return state;
}
};
}

export default queueGUIReducer;
8 changes: 5 additions & 3 deletions ui/src/reducers/remoteAccess.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const initialState = {
const INITIAL_STATE = {
// the null value is used to distinguish between signed out (null) or logged in (true/false)
sid: null,
observers: [],
Expand All @@ -8,7 +8,7 @@ const initialState = {
chatMessageCount: 0,
};

export default (state = initialState, action) => {
function remoteAccessReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_RA_STATE': {
return {
Expand Down Expand Up @@ -51,4 +51,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default remoteAccessReducer;
7 changes: 5 additions & 2 deletions ui/src/reducers/sampleChanger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable sonarjs/no-duplicate-string */
const INITIAL_STATE = {
contents: {},
state: 'READY',
Expand Down Expand Up @@ -87,7 +88,7 @@ const INITIAL_STATE = {
selectedDrop: null,
};

export default (state = INITIAL_STATE, action) => {
function sampleChangerReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_SC_CONTENTS': {
return { ...state, contents: action.data.sampleChangerContents };
Expand Down Expand Up @@ -122,4 +123,6 @@ export default (state = INITIAL_STATE, action) => {
return state;
}
}
};
}

export default sampleChangerReducer;
14 changes: 9 additions & 5 deletions ui/src/reducers/sampleGrid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const INITIAL_STATE = {
},
};

export default (state = INITIAL_STATE, action) => {
function sampleGridReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'SET_QUEUE': {
const sampleList = { ...state.sampleList };
Expand Down Expand Up @@ -122,9 +122,11 @@ export default (state = INITIAL_STATE, action) => {
const sampleIDList = action.samplesData.map((s) => s.sampleID);
const sampleList = { ...state.sampleList };
sampleIDList.forEach((sampleID, i) => {
sampleList[sampleID].tasks.length > 0
? sampleList[sampleID].tasks.concat(action.samplesData[i].tasks)
: (sampleList[sampleID].tasks = action.samplesData[i].tasks);
if (sampleList[sampleID].tasks.length > 0) {
sampleList[sampleID].tasks.concat(action.samplesData[i].tasks);
} else {
sampleList[sampleID].tasks = action.samplesData[i].tasks;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-unused-expressions — ternaries are expressions that resolve to a value, so using a ternary just to trigger side effects is counter-intuitive. if/else blocks are better suited.

This change actually reveals a more serious issue, which is that the array returned by concat is not used (concat does not mutate the arrays) — so the expression in the if block has no effect at all. Not sure what the correct behaviour should be. ESLint warns about it, so I decided to leave it as is for now.

});

return { ...state, sampleList };
Expand Down Expand Up @@ -370,4 +372,6 @@ export default (state = INITIAL_STATE, action) => {
return state;
}
}
};
}

export default sampleGridReducer;
19 changes: 6 additions & 13 deletions ui/src/reducers/sampleview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const initialState = {
const INITIAL_STATE = {
clickCentring: false,
clickCentringPoints: [],
clickCentringClicksLeft: -1,
Expand Down Expand Up @@ -43,7 +43,7 @@ const initialState = {
selectedShapes: [],
};

export default (state = initialState, action) => {
function sampleViewReducer(state = INITIAL_STATE, action = {}) {
switch (action.type) {
case 'TOOGLE_CINEMA': {
return { ...state, cinema: !state.cinema };
Expand Down Expand Up @@ -169,16 +169,7 @@ export default (state = initialState, action) => {
);
return { ...state, selectedShapes };
}
case 'CLEAR_ALL': {
return {
...state,
distancePoints: [],
clickCentringPoints: [],
gridList: [],
gridCount: 0,
selectedShapes: [],
};
}
case 'CLEAR_ALL':
case 'SET_CURRENT_SAMPLE': {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-duplicated-branches — duplicated case blocks

return {
...state,
Expand Down Expand Up @@ -225,4 +216,6 @@ export default (state = initialState, action) => {
default:
return state;
}
};
}

export default sampleViewReducer;
Loading
Loading