[WIP] Revert "Revert "Bump ESLint, prettier and stylelint"" #2056
Conversation
| @@ -175,13 +173,13 @@ const ReactionPicker = ({ | |||
| return searchEmojis(emojis, searchString); | |||
| }, [searchString, emojis]); | |||
|
|
|||
| const onCategoryClick = useCallback((category) => { | |||
| const onCategoryClick = (category) => { | |||
There was a problem hiding this comment.
What was the warning/reason for this? I guess we should declare an empty array as deps, but not sure why it warns.
There was a problem hiding this comment.
This was caused by: warning React Hook useCallback does nothing when called with only one argument. Did you forget to pass an array of dependencies? react-hooks/exhaustive-deps` eslint-plugin-react-hooks
Fixed this by passing an empty array as you suggested. 💯
| @@ -273,7 +273,7 @@ export default class Table extends Component<Props, State> { | |||
|
|
|||
| const match = Object.keys(this.state.filters).filter((key) => { | |||
| const { inlineFiltering = true, filterMapping = (val) => val } = | |||
| this.props.columns.find((col) => col.dataIndex == key) || {}; | |||
| this.props.columns.find((col) => col.dataIndex === key) || {}; | |||
There was a problem hiding this comment.
Sure this logic stays 100% similar and works if the dataIndex is a number?
There was a problem hiding this comment.
As far as i can tell, dataIndex is always a string (In our codebase). If the dataIndex is a "number", it will probably just be typeof string anyways. 🤷
| @@ -327,7 +330,7 @@ export const selectAllRegistrationsForEvent = createSelector( | |||
| updatedBy, | |||
| }); | |||
| }) | |||
| .filter((reg) => reg.event == eventId) | |||
| .filter((reg) => reg.event === eventId) | |||
There was a problem hiding this comment.
Rip int and string comparison. Fixed by wrapping eventId in parseInt()
| @@ -95,6 +95,7 @@ function loadData({ query, match: { params }, location }, dispatch) { | |||
|
|
|||
| function mapStateToProps(state, props) { | |||
| const { pathname, search } = state.router.location; | |||
| // eslint-disable-next-line no-restricted-globals | |||
There was a problem hiding this comment.
What about fixing this instead of ignoring it, since the warning is legit?
There was a problem hiding this comment.
You're right. I fixed this with window.location instead of location and ConfirmModalWithParent instead of confirm()
| @@ -44,9 +44,12 @@ const GroupMembersList = ({ | |||
| filters, | |||
| }: Props) => { | |||
| const GroupMembersListColumns = (fullName, membership) => { | |||
| console.log(groupsById); | |||
| @@ -155,6 +158,7 @@ export const getContactedStatuses = ( | |||
| ) { | |||
| contacted.splice(contacted.indexOf(status), 1); | |||
| } | |||
| return null; | |||
There was a problem hiding this comment.
Is there a reason to have a map and not a forEach here?
There was a problem hiding this comment.
Yeah, I agree. Moved these to forEach.
| '403': 'Denne siden har du ikke tilgang på', | ||
| '404': 'Denne siden finnes ikke', | ||
| '500': 'Noe gikk veldig galt, Webkom er på saken!', | ||
| // Non-string literal property keys is not supported in flow yet |
There was a problem hiding this comment.
Same here, is there a reason?
There was a problem hiding this comment.
Not an issue in the latest version of eslint.
| @@ -53,10 +53,12 @@ const OnKeyDownHandler = ({ | |||
| }) => ( | |||
| useEffect( | |||
| () => ( | |||
| // eslint-disable-next-line no-sequences | |||
There was a problem hiding this comment.
When we have to have this warning, isn't it better to just rewrite the code?
There was a problem hiding this comment.
I fixed these, but I am unsure of the () => (func1(), func2()) syntax meaning. Does this first execute func1, and then return func2? Hopefully you can review my suggested solution in my recently created PR. 🙂
| @@ -158,6 +158,7 @@ const validateMandatory = (inputAnswers: Array<Object>, props) => { | |||
| if (question.mandatory && !answeredQuestionIds.includes(question.id)) { | |||
| errors.questions[question.id] = 'Dette feltet er obligatorisk'; | |||
| } | |||
| return null; | |||
There was a problem hiding this comment.
Yup, moved to forEach. 😄
| @@ -158,6 +160,7 @@ export function deleteEntities(deleteTypes: ?EntityReducerTypes) { | |||
| ? [Number(resultId), resultId.toString()] | |||
| : [resultId]) | |||
| ), | |||
| // eslint-disable-next-line no-sequences | |||
There was a problem hiding this comment.
Maybe rewrite this instead?
There was a problem hiding this comment.
Also unsure of the syntax here 🙂, but I think the rewrite should be about right. 🙃
AdrianAndersen
force-pushed
the
revert-2055-revert-2041-bumpESLint
branch
from
57ae0ea
to
d1f5580
Compare
|
This PR should be resolved in #2084 |
Reverts #2055
EDIT: This is solved by: #2084