Skip to content

[Refactor:Developer] Strict Eslint warnings on CI#12403

Merged
bmcutler merged 11 commits intomainfrom
strict-eslint
Feb 5, 2026
Merged

[Refactor:Developer] Strict Eslint warnings on CI#12403
bmcutler merged 11 commits intomainfrom
strict-eslint

Conversation

@williamschen23
Copy link
Contributor

@williamschen23 williamschen23 commented Feb 2, 2026

Why is this Change Important & Necessary?

Several warnings that could have been fixed by developers were uncaught due to it being warnings. For example, our indenting rule was violated by some files and was only a warning.

% npm run eslint                                                                                                                       26-02-02 - 12:26:21

> eslint
> eslint .


Submitty/site/vue/src/components/Markdown.vue
  70:5  warning  'v-html' directive can lead to XSS attack  vue/no-v-html

Submitty/site/vue/src/components/Notification.vue
  83:1  warning  Expected indentation of 8 spaces but found 6 spaces  vue/html-indent
  84:1  warning  Expected indentation of 8 spaces but found 6 spaces  vue/html-indent
  85:1  warning  Expected indentation of 8 spaces but found 6 spaces  vue/html-indent
  86:1  warning  Expected indentation of 6 spaces but found 4 spaces  vue/html-indent

Submitty/site/vue/src/components/StudentInformationPanel.vue
  202:17  warning  'v-html' directive can lead to XSS attack  vue/no-v-html
  208:13  warning  'v-html' directive can lead to XSS attack  vue/no-v-html

Submitty/site/vue/src/pages/CourseNotificationsPage.vue
  16:7  warning  Attribute ':unseenCount' must be hyphenated  vue/attribute-hyphenation

Submitty/site/vue/src/pages/HomePage.vue
  127:105  warning  'class' should be on a new line                           vue/max-attributes-per-line
  137:26   warning  Variable 'course' is already declared in the upper scope  vue/no-template-shadow
  168:9    warning  Attribute ':unseenCount' must be hyphenated               vue/attribute-hyphenation

✖ 11 problems (0 errors, 11 warnings)
  0 errors and 7 warnings potentially fixable with the `--fix` option.

What is the New Behavior?

added Max warning. We need some of the warnings such as v-html as I have no idea how to ignore it. Ignoring it line by line doesnt work and ignoring the entire file doe not work either.

We can add these rules to eslint errors, but we should also limit warnings so that we can caught future warnings that should be errors as well

What steps should a reviewer take to reproduce or test the bug or new feature?

Review the code, run the command in ci.yml to see that warnings went down.

Automated Testing & Documentation

Changing CI tests

Other information

@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.67%. Comparing base (35028b0) to head (e23c592).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12403   +/-   ##
=========================================
  Coverage     21.67%   21.67%           
  Complexity     9620     9620           
=========================================
  Files           268      268           
  Lines         36164    36164           
  Branches        486      486           
=========================================
  Hits           7837     7837           
  Misses        27845    27845           
  Partials        482      482           
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.69% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.72% <ø> (ø)

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@JManion32 JManion32 left a comment

Choose a reason for hiding this comment

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

Looks great! I think this is very necessary.

I believe the issue with the no-template-shadow warning is that the file has a course prop, and then tries to introduce a variable with the same name for the loop at line 137.

Based on my testing I think you can just delete that prop entirely. It's not passed in from the PHP view, and is only a necessary prop in the child component, NotificationDisplay.vue.

@github-project-automation github-project-automation bot moved this from Seeking Reviewer to Work in Progress in Submitty Development Feb 4, 2026
@github-project-automation github-project-automation bot moved this from Work in Progress to Awaiting Maintainer Review in Submitty Development Feb 5, 2026
@cjreed121 cjreed121 moved this from Awaiting Maintainer Review to Ready to Merge in Submitty Development Feb 5, 2026
@bmcutler bmcutler merged commit 831eb19 into main Feb 5, 2026
47 of 48 checks passed
@bmcutler bmcutler deleted the strict-eslint branch February 5, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants