Skip to content
This repository was archived by the owner on Feb 6, 2026. It is now read-only.

Linting Is Back - #8276

Merged
jobelenus merged 1 commit into
mainfrom
jobelenus/linting
Jan 16, 2026
Merged

Linting Is Back#8276
jobelenus merged 1 commit into
mainfrom
jobelenus/linting

Conversation

@jobelenus

@jobelenus jobelenus commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

So What Had Happened Was

  1. We were on eslint v8, which was EOL.
  2. Most of the plugins stopped working with that.
  3. Now, we're on eslint v9.
  4. v9 changed the configuration format entirely.
  5. Both changes meant upgrading a ton of other packages as well
  6. We also upgraded typescript and pinia
  7. I got rid of the shared eslint-config package just to remove a bit of complexity, we can re-do that kind of thing if we want
  8. Some rule names changed
  9. Some new rules came in

Now—you can open the root si/ folder and get working linting. You don't need to open si/app/web or si/lib/vue-lib by itself to get linting to work.

We have some rules that I turned into warnings. I didn't want to turn them off because sometimes I want the user to see the note in case they want to take action on it. But, I also don't want them to stop a build from passing. It is a gray area.

However, there are some linting rules that are warnings (e.g. console, let not const, etc) that we don't want passing builds, but, could be really annoying in that pop-over at the bottom of the web app when developing locally?

Let's discuss!

Things I've Learned About VScode Running ESLint

  1. If you do pnpm installs, it is incredibly likely you'll need to entirely close VSCode. Simpy restarting the ESLint server does not always pick up the changed packages
  2. If you change the ESLint config restarting ESLint Server generally works reliably.

What this means for you: after this merges, the first time you work in one of the TS app or lib dirs you'll need to run pnpm i for all the dev package changes. And you will need to fully quit VScode and re-open it.

How was it tested?

  1. VSCode working!
  2. pnpm lint:fix
  3. pnpm build:check
  4. CI runs the linter & build check through buck2

Does it require a docs change?

Yes... here is what my VSCode setup looks like:

  1. You need the ESLint plugin
  2. You need the Prettier plugin
  3. This my JSON settings
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
        "source.fixAll.eslint": "always",
    },
    "eslint.workingDirectories": [{ "mode": "auto" }],
    "eslint.format.enable": true,
    "eslint.lintTask.enable": true,
    "typescript.preferences.preferTypeOnlyAutoImports": false,
    "eslint.run": "onSave",
    "eslint.lintTask.options": "--ext .ts,.js,.cjs,.vue",
    "[vue]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
    "[typescript]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
    "[javascript]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },

NOTE: I made a new VSCode profile in order to test that, and then made it the default. You may or may not want to do that

@jobelenus
jobelenus force-pushed the jobelenus/linting branch 2 times, most recently from ff475ba to 0008e5b Compare January 15, 2026 14:08
@github-actions

github-actions Bot commented Jan 15, 2026

Copy link
Copy Markdown

Dependency Review

The following issues were found:

  • ✅ 0 vulnerable package(s)
  • ⚠️ 2 packages with OpenSSF Scorecard issues.

View full job summary

@jobelenus
jobelenus force-pushed the jobelenus/linting branch 9 times, most recently from 94a86f1 to 630df87 Compare January 16, 2026 13:55
@jobelenus
jobelenus marked this pull request as ready for review January 16, 2026 14:07
export function isRawSocketEdge(edge: RawEdge): edge is RawSocketEdge {
return "fromSocketId" in edge;
}
export function isRawSubscriptionEdge(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

<3 that it allows longer lines, I felt we broke them up too much

@vbustamante vbustamante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works wonders, great fix

@jobelenus
jobelenus added this pull request to the merge queue Jan 16, 2026
@@ -121,7 +121,7 @@ const canHaveChildren = computed(() => {

const valueUrl = computed(() => {
if (!_.isString(props.value)) return null;
if ((props.value as string).match(/https?:\/\//)) {
if ((props.value).match(/https?:\/\//)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Weird that it didn't remove the unnecessary parens too

@jkeiser jkeiser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good. I looked through the .ts files and the lint config, and it's really good! Very happy with line length being smaller :)

I think we need to make sure warnings don't make their way into main, and it looks like this won't detect that? But this is way less painful than our existing situation.

We should follow up fast with something that prevents warnings in main, but not block this and let your work rot.

Merged via the queue into main with commit d3ffffd Jan 16, 2026
11 checks passed
@jobelenus
jobelenus deleted the jobelenus/linting branch January 16, 2026 18:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants