Skip to content

fix: template mobile-nav set-state-in-effect + lint-staged unmatched-pattern - #539

Merged
MarioCadenas merged 2 commits into
mainfrom
fix/template-mobile-nav-set-state-in-effect
Aug 18, 2026
Merged

fix: template mobile-nav set-state-in-effect + lint-staged unmatched-pattern#539
MarioCadenas merged 2 commits into
mainfrom
fix/template-mobile-nav-set-state-in-effect

Conversation

@MarioCadenas

Copy link
Copy Markdown
Collaborator

What

Two small lint-correctness fixes, both surfaced while regression-testing the Biome→oxc migration (#538) by scaffolding and linting a fresh app:

  1. template/client/src/App.tsx — the mobile-nav Sheet reset its open state inside a useEffect keyed on isMobile, which eslint-plugin-react-hooks v7 flags as react-hooks/set-state-in-effect. Fixed by deriving the open prop (open={mobileNavOpen && isMobile}) and dropping the effect.
  2. package.json lint-staged — restore --no-errors-on-unmatched behavior (lost in the oxc migration) so commits touching only oxc-ignored files don't fail pre-commit.

Why

  • App.tsx: This eslint error has been latent since the responsive-nav change (feat: improve template and component responsiveness #391) — eslint-plugin-react-hooks has been v7 (which added set-state-in-effect) since the template was created. It went unnoticed because CI never lints the template or a scaffolded app. The fix follows React's recommended pattern (compute during render instead of resetting state in an effect):

    -import { useState, useEffect } from 'react';+import { useState } from 'react';
    ...
    - // Close mobile nav when viewport crosses to desktop- useEffect(() => {- if (!isMobile) setMobileNavOpen(false);- }, [isMobile]);
    ...
    - <Sheet open={mobileNavOpen} onOpenChange={setMobileNavOpen}>+ <Sheet open={mobileNavOpen && isMobile} onOpenChange={setMobileNavOpen}>

    Behavior is preserved: on desktop the portaled Radix sheet is forced closed; on mobile the toggle works; a mobile→desktop resize closes it and Radix fires onOpenChange(false), keeping state in sync — no effect needed.

  • lint-staged: oxlint/oxfmt exit non-zero on an all-ignored fileset ("No files found to lint"). The pre-oxc Biome command used --no-errors-on-unmatched; this restores that via --no-error-on-unmatched-pattern on both tools.

Verification

On a freshly scaffolded app (databricks apps init from the template):

  • eslint .0 problems (was 1 error)
  • tsc -b (server + client) → passes
  • vite build → succeeds

Not fixed here (follow-up)

The root gap is that CI doesn't lint the template or a scaffolded app, so this class of error stays invisible. A CI step that scaffolds an app and runs npm run lint would catch it going forward — happy to do that separately.

oxlint/oxfmt exit non-zero on an all-ignored fileset ("No files found to
lint"), so any commit touching only files under template/, generated
outputs, etc. failed pre-commit. The former Biome command used
--no-errors-on-unmatched; restore that behavior with
--no-error-on-unmatched-pattern on the oxlint and oxfmt lint-staged steps.
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
… nav
The mobile-nav Sheet reset its open state inside a useEffect keyed on
isMobile, which eslint-plugin-react-hooks v7 flags as set-state-in-effect.
Derive the open prop instead (open={mobileNavOpen && isMobile}) so the
portaled Radix sheet can't linger on desktop, and drop the effect. Radix
fires onOpenChange(false) when open goes false, so state stays in sync.
Verified on a freshly scaffolded app: eslint clean, typecheck passes,
vite build succeeds. Latent since #391; CI never lints the template or a
scaffolded app, so it went unnoticed until a fresh scaffold was linted.
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
@MarioCadenas
MarioCadenas requested a review from a team as a code ownerAugust 18, 2026 08:59
@github-actions

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 32119194591 -R databricks/appkit -n appkit-template-0.61.1-pr.9fb8285-fix-template-mobile-nav-set-state-in-effect-539 -D appkit-pr-539 \
&& unzip -o "appkit-pr-539/appkit-template-0.61.1-pr.9fb8285-fix-template-mobile-nav-set-state-in-effect-539.zip" -d "appkit-pr-539" \
&& databricks apps init --template "appkit-pr-539"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

@MarioCadenas
MarioCadenas enabled auto-merge (squash) August 18, 2026 09:28

@pkosiecpkosiec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@MarioCadenas
MarioCadenas merged commit 3211de2 into mainAug 18, 2026
15 of 16 checks passed
@MarioCadenas
MarioCadenas deleted the fix/template-mobile-nav-set-state-in-effect branch August 18, 2026 09:29
Sign up for freeto 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

@MarioCadenas@pkosiec