chore(eslint): add unicorn/no-negated-condition rule
#2876
Conversation
csouchet
added
chore
|
♻️ PR Preview e9ca6e7 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
|
♻️ PR Preview e9ca6e7 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
csouchet
force-pushed
the
2742-add_unicorn/no-negated-condition_rule
branch
from
3d01c9d
to
e9ca6e7
Compare
csouchet
removed
the
depends on another PR ⚠️
|
Kudos, SonarCloud Quality Gate passed! |
| @@ -30,7 +30,7 @@ export class BpmnCellRenderer extends mxgraph.mxCellRenderer { | |||
| dict = new mxgraph.mxDictionary<mxShape>(); | |||
|
|
|||
| for (const currentOverlay of overlays) { | |||
| const shape = state.overlays != null ? state.overlays.remove(currentOverlay) : null; | |||
| const shape = state.overlays == null ? null : state.overlays.remove(currentOverlay); | |||
There was a problem hiding this comment.
question: this is mainly a warning for the next estlint rules we will enabled. The question is: "@csouchet do do you agree with the following statement?"
Here, we modify some original mxGraph code. Up to now, we've tried to modify the code only when necessary. This was initially done to manage mxGraph version changes (know what we've changed, so if the original mxGraph code changes, we can more easily backport our changes to the new implementation).
With mxGraph at the end of its life, this is probably no longer a strong requirement. However, if we decide to switch to maxGraph, it may be useful for the initial migration. The ability to modify maxGraph will be resurrected.
Here, we're starting to change something that isn't necessary, except to match our linting rules (we've done it before, but that was for indentation, or switching to TypeScript, but only very slight things).
The change here doesn't alter the effect of the implementation: it just reverses the condition. So, in this case, I think it's safe to apply the change.
This may not be the case for other types of changes due to the next unicorn rules we may activate.
There was a problem hiding this comment.
@tbouffard hadn't paid attention to the changes here, but indeed, for other rules, I disabled them on the mxgraph code, because I didn't know the impact, for example replacing null with undefined.
| if (off == 0) { | ||
| this.node.removeAttribute('transform'); | ||
| } else { | ||
| this.node.setAttribute('transform', 'translate(' + off + ',' + off + ')'); | ||
| } |
There was a problem hiding this comment.
thought: the change updates the original code of mxGraph. See my remark about src/component/mxgraph/BpmnCellRenderer.ts
To avoid to have to many changes by enabling plugin:unicorn/recommended, I choose to enable some rule one-by-one.
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-negated-condition.md
Part of #2824
Covers #2742