# PR: chore: improve ESLint & Prettier config + fix linting issues (fixes #80)

[glenjaysondmello]

Summary

This PR tightens and modernizes the repository's linting & formatting setup, fixes real code issues revealed by the stricter rules, and updates CI to run a combined lint + format check.
Key outcomes:

• Add robust ESLint config (.eslintrc.cjs) and Prettier config (prettier.config.cjs) that support modern ES (ES2023) and ESM projects.
• Add .eslintignore and .prettierignore (safety layer even though ignore patterns exist in configs).
• Update package.json scripts to include lint:ci and make lint dev-friendly.
• Update CI workflow to run npm run lint:ci.
• Fix real code issues discovered by linting/formatting (email regex, duplicated search listener/indentation issues, tag rendering change).
• Run Prettier across repository and include resulting formatting changes.

This addresses issue #80: improved Prettier and ESLint rules, and ensures both local/manual runs and GitHub Actions behave consistently.

---

PR body (copy / paste into GitHub PR description)

What I changed

Configuration & tooling

• Added .eslintrc.cjs — main ESLint config (CommonJS) that:

  • Uses @babel/eslint-parser to support modern JS/JSX without requiring a separate Babel config.
  • Enables plugin:prettier/recommended to run Prettier as an ESLint rule and disable conflicting ESLint formatting rules.
  • Adds ignorePatterns for generated files.
  • Uses a dynamic no-console rule (dev-friendly).

• Added .eslintignore — repository-level ignore file (safety net for editors, pre-commit hooks, and broken config scenarios).

• Kept an upgraded .eslintrc.json (present but will be unused while .eslintrc.cjs exists) to preserve a JSON option if needed.

• Added prettier.config.cjs and .prettierrc (kept compatible). prettier.config.cjs allows comments and JS-level future extensibility.

• Added .prettierignore.

• Updated package.json scripts:

  • "lint": "eslint \"**/*.{js,jsx,mjs,cjs}\"" (developer mode, no --max-warnings 0)
  • "lint:fix": "eslint \"**/*.{js,jsx,mjs,cjs}\" --fix"
  • "check:format": "prettier --check ."
  • "lint:ci": "npm run lint && npm run check:format" (used by CI)

• Installed/added devDependencies required for the parser & plugins (see package.json in this PR): @babel/eslint-parser, eslint, eslint-config-prettier, eslint-plugin-import, eslint-plugin-prettier, prettier.

CI

• Replaced the original CI steps:

  - name: Lint Code
    run: npm run lint
  
  - name: Check Formatting
    run: npx prettier --check .

  with the single combined step:

  - name: Lint & Format Check
    run: npm run lint:ci

  This reduces noise and makes the workflow clearer.

Code fixes discovered during linting

• src/models/User.js:

  • Fixed email regex from /.+\@.+\..+/ → /.+@.+\..+/ (remove unnecessary escape \@). This removes the no-useless-escape violation and is the correct JS regex.

• src/js/main.js (stream grid rendering / search logic):

  • Removed duplicated searchInput listener and kept the debounced search only.

  • Extracted tag HTML building into tagsHtml before the template to satisfy the indent rule and improve readability:

    const tagsHtml = (stream.tags || [])
      .map(tag => `<span class="badge ...">${tag}</span>`)
      .join('');

  • Replaced inline multi-line .map() inside template literal with ${tagsHtml} to avoid ESLint indent complaints.

  • Fixed inconsistent indentation across the file.

• Misc: Prettier formatted docs and static files (see list of modified files below).

---

Why these changes? (rationale & benefits)

.eslintrc.cjs and prettier.config.cjs (why CJS)

• The repo uses "type": "module" in package.json (ESM). Node treats .js as ESM in that context and ESLint historically expects CommonJS configs. .cjs forces CommonJS and is stable across environments.
• .cjs files allow comments, conditional logic, and dynamic config (for example, process.env.CI checks to set stricter rules in CI). JSON config cannot do that.
• Using JS config makes the setup future-proof (able to add environment-specific rules, import other configs, or programmatically compose configs).
  Here is the consolidated point:
• Configuration tools follow a strict priority hierarchy where specific files override others: .eslintrc.cjs takes precedence over (and ignores) .eslintrc.json, whereas a static .prettierrc file takes priority over (and ignores) prettier.config.cjs.

---

.eslintignore in addition to ignorePatterns

• ignorePatterns is config-level and is only read after ESLint loads the config. .eslintignore is read earlier and is respected by editors, CLI wrappers, lint-staged, and is a safety net if config loading fails.
• Keeping both is standard practice in many professional repos.

lint script & --max-warnings approach

• During development we use a forgiving lint script (no --max-warnings 0) so contributors can iterate locally without CI-level strictness stopping them.
• In production / CI we will enable --max-warnings 0 (or set no-console to error in CI) to enforce zero warnings. For this repo we introduced lint:ci so CI will run a stricter combined check; when the team decides to flip the final switch we can change lint or add --max-warnings 0 in package.json or run it in CI.
• This staged approach avoids failing contributor PRs for warnings while still enabling a path to zero-tolerance in production.

Combined CI step

• Running npm run lint:ci groups lint and format-check into one single step for clarity and better logs. It also reduces risk of duplicative steps and keeps the workflow tidy.

---

Files added / modified (high level)

Added (untracked before):

• .eslintignore
• .eslintrc.cjs
• .prettierignore
• prettier.config.cjs

Modified (Prettier / ESLint / code changes):

• .eslintrc.json (upgraded JSON form)
• .prettierrc
• package.json (scripts + dev deps)
• package-lock.json
• .github/workflows/ci.yml (use npm run lint:ci)
• .env.example (Added NODE_ENV and JWT_SECRET to the template)
• CODE_OF_CONDUCT.md, CONTRIBUTING.md, DEVSETUP.md, README.md (formatting)
• index.html, src/css/style.css, streams.json (formatted)
• server.js, src/controllers/AuthControllers.js, src/middleware/auth.js (minor formatting & lint fixes)
• src/js/main.js (search & stream rendering fixes: dedupe search listener, indentation, tagsHtml extraction)
• src/models/User.js (email regex fix)
• vite.config.js (formatting)

See the full git diff for exact edits.

---

How I tested this locally

Run these commands in project root:

# fresh install
npm ci

# run Prettier and ESLint autofixes locally
npm run format
npm run lint:fix

# run lint to ensure no errors (warnings allowed in dev)
npm run lint

# run the CI check locally to mimic CI step
npm run lint:ci

Expected result locally:

• npm run format will apply formatting changes across the repo (many files will be updated).
• npm run lint:fix will auto-fix fixable ESLint issues.
• npm run lint in development mode (as configured) should not fail for warnings because lint is currently configured without --max-warnings 0.
• npm run lint:ci runs both lint and prettier --check . and should succeed once code is fixed and formatted.

---

Notes / follow-ups for maintainers

• Production enforcement: once you want CI to treat warnings as failures, switch lint back to include --max-warnings 0 or set no-console to error in CI (e.g., process.env.CI check inside .eslintrc.cjs).

• Husky + lint-staged: I recommend adding husky + lint-staged to run eslint --fix + prettier --write on staged files to prevent bad commits.

• Monorepo / shared config: If you later adopt a monorepo, consider migrating to ESLint flat config (eslint.config.js) or centralizing config in a shared package.

• Testing: No runtime behavior changes were introduced except for bug fixes (regex, search duplication removal, and tag rendering). Please sanity-check the streams UI and user signup flow locally.

• Config format preference: currently both JSON and CJS configs are present:

  • ESLint: .eslintrc.cjs and .eslintrc.json
  • Prettier: prettier.config.cjs and .prettierrc

  .cjs is the active/authoritative config (and is required for ESM + comments + dynamic logic), while the JSON versions are kept for backward compatibility/readability.

  Please let me know:

  • Should we keep both, or
  • Remove the JSON files and standardize on .cjs only?

  I can clean this up based on your preference.

---

How reviewers can validate quickly

1. Checkout this branch.

2. npm ci

3. npm run format

4. npm run lint:fix

5. npm run lint (should pass in dev mode)

6. npm run lint:ci (this should pass if all formatting/lint fixes are applied)

7. Load the app (npm run dev or node server.js) and verify:

   • Streams grid renders tags and details correctly
   • Signup/login forms accept valid emails and do not fail due to regex
   • No JS console errors from the changes performed

---

Final note

This PR is intentionally conservative: it improves developer experience locally while establishing a clear path to stricter, zero-warning CI in production. The .cjs configs are future-proof and allow flexible, environment-aware rules; .eslintignore is kept as a safe, ecosystem-level ignore file. If maintainers prefer JSON-only configs, we can convert back, but we’ll lose the ability to use comments and programmatic logic in configs.

[gbowne1]

@glenjaysondmello

This is great however theres already a PR to fix the .env-example filename. Please see: #81 #84 #72

[shishir-21]

Thanks for flagging this 👍

I went through PR #87 in detail. The intent to improve ESLint/Prettier consistency and address #80 is good, and some changes (like the Windows-safe .env.example rename and CI cleanup) definitely help.

That said, this PR is quite large and mixes multiple concerns — lint/config updates, code logic refactors (notably in AuthControllers), frontend formatting, docs, and generated files like package-lock.json. Given the size and scope, it feels a bit risky to merge quickly, especially while we’re already dealing with CI instability.

I agree with your concern — if the underlying issue (e.g. #78) isn’t addressed first, we’re likely to keep running into the same problems. It might be safer to tackle the root fix and then bring these improvements in smaller, focused PRs.

Happy to re-review anything once things are split or once #78 is addressed.

[gbowne1]

@shishir-21

I agree however I would simply approve and merging this as it clearly addresses the entire issues we are having then your PR can simply refine this on top of that merge. Sure it will require ANOTHER rebase but it's a relatively small price to pay for a big CI fix, etc.

I can use the UI to fix the single env conflict

I also want to get in a few of the features that were added like chat, going live, etc by #61 etc

[gbowne1]

✅ rebase after conflict still passed

All good from here. 💯

This is indeed the fix for this

[shishir-21]

@gbowne1

Thanks for the clarification 👍

That makes sense. If merging #87 helps stabilize CI end-to-end, I’m on board with that approach.
I’m happy to rebase and refine my PR on top of that merge once things settle.

Appreciate you handling the env conflict as well.

[shishir-21]

@gbowne1

Awesome, thanks for checking and confirming 🙌
Appreciate the quick review!

[gbowne1]

You're doing just fine. No worries about it

We can nit pick and further refine CI with your PR. I don't think your PR needs closed just further refined fixes.

Giving this some thought, looking at the ✅ passed run seems like it

Of course time will tell but that run highlight an issue with separated concerns. The next round should be much easier.

Probably a good place to refine the Development documentation and README.

[shishir-21]

ready to merge.

Thank you.

[gbowne1]

Might add some details to:

https://github.com/gbowne1/codestream/blob/0c28bf443fac8d35bf031f589efb390e65749033/CONTRIBUTING.md

[gbowne1]

@glenjaysondmello

I checked out this change locally for testing and review for merge

This PR correctly fixes the broken CI run plus correctly fixed the identified issues with the files in the repository from lint and formatting issues.

Approving this PR for merge pending futher review by collaborators and maintainers

Thanks for the opportunity to review your PR and for your contribution to this project

[gbowne1]

I think the package-lock.json could be removed then regenerated after merge or we delete it and add it to git ignore along with node_modules directory.

Then the file and directory would only exist post installation by the user.

package-lock.json got messed up somehow

Thoughts on how to resolve this?

@glenjaysondmello @shishir-21 @Ved178

[glenjaysondmello]

I think the package-lock.json could be removed then regenerated after merge or we delete it and add it to git ignore along with node_modules directory.

Then the file and directory would only exist post installation by the user.

package-lock.json got messed up somehow

Thoughts on how to resolve this?

@glenjaysondmello @shishir-21 @Ved178

keep package-lock.json, regenerate a clean lockfile locally using the same Node version as CI, verify the repo’s lint/format checks locally, then commit the regenerated package-lock.json. This preserves reproducible installs and avoids surprise dependency changes.

[gbowne1]

@glenjaysondmello

Post merge or now?

[glenjaysondmello]

@glenjaysondmello

Post merge or now?

Post-merge is better.

[gbowne1]

@Ved178 @shishir-21 @glenjaysondmello @Haseebx162006 can you review this? I think this should be the next next PR merged.

Took a day off to collect some thoughts

We will have to fix the package-lock file but that should be a side from merging this

I view this is sort of a Emergency fix to the problems we've been experiencing with multiple contributes to the same set of files

[Haseebx162006]

@gbowne1 okk i will review this PR

[shishir-21]

@gbowne1
The package-lock.json changes are very large for a lint/formatting PR. I agree we should keep the lockfile, but its regeneration/cleanup should be handled in a dedicated follow-up PR using the CI Node/NPM versions so it doesn’t block this merge.

[gbowne1]

Yeah somehow that went not so good. I think this PR should go first next, but 🤷‍♂️

[shishir-21]

@gbowne1
Agreed
This should go first. We can handle the lockfile cleanup separately.

[gbowne1]

It should save a little bit of work once we expand the main.js and server.js from being monolithic

[gbowne1]

Merging this now as an emergency WIP towards goal of fixing CI and a refactoring.

@shishir-21 @Ved178 @Haseebx162006

The package-lock.json. from here will need to be fixed asap