Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 10, 2025

Add lint-staged configuration with pre-commit hooks for src/ directory

Summary

This PR adds lint-staged configuration to automatically format and lint TypeScript files in the src/ directory on pre-commit. This is the first of two PRs to sync code quality tooling with the main deepnote/deepnote repository.

Key changes:

  • Added husky (9.1.7) and lint-staged (16.2.3) dependencies matching deepnote/deepnote versions
  • Configured lint-staged to run eslint --cache --fix and prettier --write on src/**/*.{ts,tsx} files only
  • Added three Husky git hooks:
    • pre-commit: runs lint-staged on staged files
    • post-checkout: runs jlpm install to keep dependencies in sync
    • pre-push: blocks direct pushes to main branch
  • Added .venv to .prettierignore to prevent prettier from checking Python virtual environment files

Review & Testing Checklist for Human

  • Test pre-commit hook functionality: Make a change to a .ts file in src/, stage it, and verify the hook runs eslint and prettier automatically on commit
  • Verify jlpm availability: Confirm that jlpm exec lint-staged works in git hook context (the CodeRabbit bot raised a concern about this)
  • Test src/ directory scoping: Verify lint-staged only processes files in the src/ directory and ignores other TypeScript files in the project root

Notes

  • This PR intentionally keeps existing ESLint/Prettier configurations unchanged
  • The next PR will sync biome.json, prettier, and tsconfig settings from deepnote/deepnote for src/ directory only
  • All yarn.lock changes are legitimate transitive dependencies from husky and lint-staged packages

Link to Devin run: https://app.devin.ai/sessions/145f89c8cbc942d2970c5bc43a90149c
Requested by: @jamesbhobbs

- Add husky (9.1.7) and lint-staged (16.2.3) dependencies
- Configure lint-staged to run eslint and prettier on src/**/*.{ts,tsx} files
- Add pre-commit hook to run lint-staged
- Add post-checkout hook to run yarn install
- Add pre-push hook to prevent direct pushes to main
- Add .venv to .prettierignore to fix lint:check
@devin-ai-integration
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

📝 Walkthrough

Walkthrough

Adds Husky integration: a prepare script in package.json, husky and lint-staged as devDependencies, and a top-level lint-staged config to run ESLint/Prettier on TypeScript files. Introduces Husky hooks: .husky/post-checkout runs jlpm install, .husky/pre-commit runs jlpm lint-staged on staged files, and .husky/pre-push blocks pushes to the main branch. Adds .venv to .prettierignore.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant Git as Git
  participant Husky as Husky Hooks
  participant LStaged as lint-staged
  participant JLPM as jlpm

  rect rgb(235, 245, 255)
    note over Dev,Git: Checkout flow
    Dev->>Git: git checkout <branch>
    Git->>Husky: trigger post-checkout
    Husky->>JLPM: jlpm install
  end

  rect rgb(240, 255, 240)
    note over Dev,Git: Commit flow
    Dev->>Git: git commit
    Git->>Husky: trigger pre-commit
    Husky->>LStaged: jlpm lint-staged (run linters/formatters on staged TS files)
    LStaged-->>Husky: exit code
    Husky-->>Git: allow/deny commit
  end

  rect rgb(255, 245, 235)
    note over Dev,Git: Push flow
    Dev->>Git: git push
    Git->>Husky: trigger pre-push
    Husky->>Husky: read current branch
    alt branch is "main"
      Husky-->>Git: exit 1 (block push)
    else other branch
      Husky-->>Git: exit 0 (allow push)
    end
  end
Loading

Possibly related PRs

  • deepnote/deepnote-internal#18551: Overlaps .husky/post-checkout changes (install behavior).
  • deepnote/deepnote-internal#18158: Modifies Husky hook scripts (.husky/pre-commit, .husky/pre-push), overlapping hook behavior.
  • chore: Set up Biome for linting and formatting. deepnote#9: Updates package.json tooling (prepare/husky and lint-staged) and formatting ignore rules, overlapping config changes.

Suggested reviewers

  • Artmann
  • saltenasl
  • andyjakubowski

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the main change—adding lint-staged and a pre-commit hook for the src directory—which aligns with the PR’s primary objective.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.50%. Comparing base (ec81dd0) to head (3f86f3e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #18   +/-   ##
=======================================
  Coverage   18.50%   18.50%           
=======================================
  Files          13       13           
  Lines         200      200           
  Branches       27       27           
=======================================
  Hits           37       37           
  Misses        163      163           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 10, 2025
- Update pre-commit hook to use 'jlpm exec lint-staged'
- Update post-checkout hook to use 'jlpm install'
- Aligns with jupyterlab-deepnote's package manager setup
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9584dd8 and 03048b0.

📒 Files selected for processing (2)
  • .husky/post-checkout (1 hunks)
  • .husky/pre-commit (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T13:34:45.847Z
Learnt from: jamesbhobbs
PR: deepnote/deepnote#1
File: .husky/post-checkout:1-1
Timestamp: 2025-09-30T13:34:45.847Z
Learning: In the deepnote/deepnote repository, the .husky/post-checkout hook should remain minimal (just `pnpm install`) without explicit flags like `--frozen-lockfile` or guards. Explicit flags should only be used where needed, such as in CI workflows.

Applied to files:

  • .husky/post-checkout
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: check_release
  • GitHub Check: build
🔇 Additional comments (1)
.husky/post-checkout (1)

1-1: Performance impact on frequent checkouts.

Running install on every checkout (branch switch, pull, etc.) may slow down developer workflow, especially when dependencies haven't changed. Based on learnings, keeping the hook minimal is preferred.

Consider monitoring the performance impact in practice. If slowdowns occur, users can selectively bypass with --no-verify on checkouts or consider conditional execution based on package file changes.

The packageManager field was automatically added by yarn during
initial development but should not be present since jupyterlab-deepnote
uses jlpm (JupyterLab's yarn wrapper) for package management.
@jamesbhobbs jamesbhobbs changed the title Add lint-staged with pre-commit hooks for src/ directory chore: add lint-staged with pre-commit hooks for src/ directory Oct 10, 2025
@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 10, 2025 14:59
@jamesbhobbs jamesbhobbs merged commit 88fd175 into main Oct 14, 2025
12 checks passed
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.

4 participants