bpmn-io / diagram-js

A toolbox for displaying and modifying diagrams on the web.
MIT License
1.71k stars 418 forks source link

Migrate to ESLint 9 and flat config #936

Closed philippfromme closed 1 month ago

philippfromme commented 1 month ago

Related to https://github.com/bpmn-io/internal-docs/issues/1042

Proposed Changes

### Checklist To ensure you provided everything we need to look at your PR: * [x] **Brief textual description** of the changes present * [ ] **Visual demo** attached * [ ] **Steps to try out** present, i.e. [using the `@bpmn-io/sr` tool](https://github.com/bpmn-io/sr) * [x] Related issue linked via `Closes {LINK_TO_ISSUE}` or `Related to {LINK_TO_ISSUE}`
nikku commented 1 month ago

I've update this PR to use eslint-plugin-bpmn-io@2, and it found additional violations that are reasonable:

image

Some minor adjustments for the mocha validations were needed, I propose to incorporate them in the plug-in in a patch version.

My investigation is completed for the moment, @philippfromme consider when and how to take this back over.

barmac commented 1 month ago

The nested test looks like a false positive. We should change the function name only.

nikku commented 1 month ago

Updated to eslint-plugin-bpmn-io@2.0.1.

philippfromme commented 1 month ago

So I guess we want to fix things like no-identical-title and not disable that rule? We have the same issue in the Camunda Modeler where we would have to fix a couple of things.

nikku commented 1 month ago

@philippfromme Yes. As mentioned in https://github.com/bpmn-io/diagram-js/pull/936#issuecomment-2404679462 I think the left over errors are quality of life.

nikku commented 1 month ago

@philippfromme Updated this PR as discussed with proper configuration of browser vs. node environments (https://github.com/bpmn-io/diagram-js/pull/936/commits/ce60276a25f53cadd32397b5d552e92dbf7ee355). After you do it, then you see that https://github.com/bpmn-io/diagram-js/pull/936/commits/85027fef783bf52e20cd49d94d8f36a24a26ff39 is indeed necessary. No-one but us defines that global.

nikku commented 1 month ago

Updated to designated eslint format: https://github.com/bpmn-io/diagram-js/pull/936/commits/4b179558d18867c8d8b7da26f22c00f985dc1aac.

nikku commented 1 month ago

@philippfromme Some open questions on this PR.

philippfromme commented 1 month ago

@nikku The deleted tests were duplicates.