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.

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

ESLint V9 - Typed #5608

Merged
merged 13 commits into from
Jun 30, 2024
Merged

ESLint V9 - Typed #5608

merged 13 commits into from
Jun 30, 2024

Conversation

sidharthv96
Copy link
Member

📑 Summary

Add typed rules check to ESLint v9.

📋 Tasks

Make sure you

Copy link

codecov bot commented Jun 29, 2024

Codecov Report

Attention: Patch coverage is 2.60870% with 224 lines in your changes missing coverage. Please review.

Project coverage is 5.86%. Comparing base (c8a3290) to head (55bd9e6).

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           sidv/eslintv9   #5608      +/-   ##
================================================
- Coverage           5.86%   5.86%   -0.01%     
================================================
  Files                274     274              
  Lines              41083   41085       +2     
  Branches             488     488              
================================================
- Hits                2409    2408       -1     
- Misses             38674   38677       +3     
Flag Coverage Δ
unit 5.86% <2.60%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
packages/mermaid/src/defaultConfig.ts 46.37% <100.00%> (ø)
packages/mermaid/src/diagram-api/diagramAPI.ts 57.83% <100.00%> (-1.00%) ⬇️
packages/mermaid/src/tests/MockedD3.ts 80.27% <100.00%> (ø)
.esbuild/build.ts 0.00% <0.00%> (ø)
.esbuild/server.ts 0.00% <0.00%> (ø)
...ckages/mermaid-example-diagram/src/mermaidUtils.ts 0.00% <0.00%> (ø)
packages/mermaid-zenuml/src/mermaidUtils.ts 0.00% <0.00%> (ø)
packages/mermaid-zenuml/src/zenumlRenderer.ts 0.00% <0.00%> (ø)
.../mermaid/scripts/create-types-from-json-schema.mts 0.00% <0.00%> (ø)
packages/mermaid/src/Diagram.ts 0.00% <0.00%> (ø)
... and 45 more

Copy link

argos-ci bot commented Jun 29, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Jun 30, 2024, 9:33 AM

Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

I've got some comments on things to potentially change in the PR.

I think I've noticed a potential bug in the extractWrap function you made, if text = '' (empty string), but 🤷, I don't know if an empty string will ever be used on that function.

Feel free to fix or ignore them and merge!

Comment on lines +122 to 123
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
return 'translate(' + labelArcGenerator.centroid(datum) + ')';
Copy link
Member

Choose a reason for hiding this comment

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

Should we switch this to a template string instead?

Suggested change
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
return 'translate(' + labelArcGenerator.centroid(datum) + ')';
return `translate(${labelArcGenerator.centroid(datum)})`;

Copy link
Member Author

@sidharthv96 sidharthv96 Jun 30, 2024

Choose a reason for hiding this comment

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

The problem is that the .centroid function returns [number, number]. So we should look into this properly, maybe when it creates an issue. Even if we use template string, it'll throw another error.

Comment on lines +118 to 119
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
textElem.attr('x', Math.round(startx + Math.abs(startx - stopx) / 2 - dim.width / 2));
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it might be a bug to me.

Are one of these values potentially not a number?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's because msgModel has a default of null, so these values have an inferred type of never.
Adding types should fix it.

sidharthv96 and others added 4 commits June 30, 2024 14:31
* sidv/eslintv9:
  chore: Log granular rebuild times
  Separate eslint packages from updates
Co-authored-by: Alois Klink <alois@aloisklink.com>
Co-authored-by: Alois Klink <alois@aloisklink.com>
@sidharthv96 sidharthv96 merged commit b27d83e into sidv/eslintv9 Jun 30, 2024
21 of 22 checks passed
@sidharthv96 sidharthv96 deleted the sidv/eslintv9Typed branch June 30, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants