chore: adopt Oxlint for JS/TS semantics; keep Biome for format/non-JS (#321) - #371
chore: adopt Oxlint for JS/TS semantics; keep Biome for format/non-JS (#321)#371thedavidweng wants to merge 5 commits into
Conversation
Adopt Oxlint 1.74 for JS/TS/React/Vitest/import semantics with type-aware floating-promise and import-cycle checks. Keep Biome as the sole formatter and non-JS linter (CSS/JSON). Update scripts, docs, and fix correctness findings surfaced by the new lint path. Co-authored-by: Davy <thedavidweng@users.noreply.github.com>
…icker-dialog Add oxlint-enable after the drop-zone section so jsx-a11y/no-noninteractive-element-interactions is not silenced for the rest of each file.
|
@thedavidweng is attempting to deploy a commit to the open-slide Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe workspace now uses Oxlint for JavaScript and TypeScript linting and Biome for formatting and non-JavaScript linting. Configuration, documentation, accessibility labels, promise handling, hook dependencies, and related lint-driven code and test updates were added. ChangesLinting split and remediation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/app/components/asset-view.tsx (1)
1384-1405: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid stale closures hidden by hook-dependency suppressions.
Both effects suppress missing dependencies while still reading values that can change during the mount.
packages/core/src/app/components/asset-view.tsx#L1384-L1405:t.asset.toastSearchFailedis locale-dependent and used inside the search error path; keept’s relevant locale state/derived messages in dependencies or serialize a stable error code.packages/core/src/app/components/sidebar/sidebar.tsx#L113-L126:commitCreate()still readst,onCreate, and the folder icon state from its render closure, while the listener dependency includes onlycreating; keept/onCreateupdated via ref or dependencies, or use refs for the locale toast values incommitCreate().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/app/components/asset-view.tsx` around lines 1384 - 1405, Update the effect around searchSvgl in packages/core/src/app/components/asset-view.tsx:1384-1405 to avoid the dependency suppression by tracking the locale-dependent t.asset.toastSearchFailed value or a stable error code, while preserving the existing search cancellation behavior. Update commitCreate and its listener in packages/core/src/app/components/sidebar/sidebar.tsx:113-126 so changing t, onCreate, or the folder icon state cannot leave the listener with stale render-closure values; use appropriate dependencies or refs for these values.
🧹 Nitpick comments (1)
packages/core/src/app/components/inspector/asset-picker-dialog.tsx (1)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the Oxlint suppression to the drop zone.
The disable begins before
<Dialog>and remains active through the entire dialog, so it can suppress future accessibility violations on unrelated controls. Use the narrowest element- or next-line-scoped directive supported by Oxlint around the<section>drop zone only.Also applies to: 191-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/app/components/inspector/asset-picker-dialog.tsx` at line 60, Narrow the jsx-a11y/no-noninteractive-element-interactions suppression from the file-level directive to the `<section>` drop zone in the asset picker dialog. Use Oxlint’s element- or next-line-scoped directive immediately around that section, and remove the broad suppression so unrelated `<Dialog>` controls remain linted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/oxlint-biome-split.md:
- Line 5: Update the changeset description in oxlint-biome-split.md to be short,
direct, present-tense, and user-facing, describing the released `@open-slide/core`
improvements rather than internal Oxlint findings; mention editor accessibility
and async operation reliability.
In @.oxlintrc.json:
- Around line 41-46: Restrict the accessibility rule overrides in the .oxlintrc
configuration from the broad slides and landing directory globs to the specific
drop-zone component files that require them. Preserve the three disabled rules
for those targeted files while ensuring navigation and other landing components
remain subject to Oxlint accessibility checks.
In `@AGENTS.md`:
- Line 39: Update the validation guidance in AGENTS.md so pnpm check:fix is
described only as a preparatory formatting/lint-fix command, followed by pnpm
check for typechecking and final validation; do not present check:fix as an
alternative to pnpm check.
- Around line 27-29: Update the lint ownership documentation to match
biome.json’s disabled CSS linting: in AGENTS.md lines 27-29, CONTRIBUTING.md
line 56, and README.md line 91, replace CSS/JSON and non-JavaScript claims with
the actual JSON/non-JavaScript scope while preserving the existing command
descriptions.
In `@biome.json`:
- Around line 31-33: Update the JavaScript/TypeScript override in biome.json so
Biome’s security lint rules remain enabled, or configure equivalent Oxlint
security rules for the same file patterns. Ensure checks covering
dangerouslySetInnerHtml, dangerouslySetInnerHtmlWithChildren, and noGlobalEval
are preserved rather than disabled by the linter.enabled setting.
In `@packages/core/src/app/components/asset-view.tsx`:
- Around line 1135-1141: Handle promise rejections in RenameAsset.commit() and
InlineTitleEditor.commit() before setSaving(false), using the existing error
logging or retry behavior as appropriate. Apply the fix to both
commit-triggering handlers in packages/core/src/app/components/asset-view.tsx
(lines 1135-1141 and 1185-1191) and the corresponding handler in
packages/core/src/app/routes/slide.tsx (lines 1105-1112); each site must avoid
unhandled rejections from void commit() calls.
In `@packages/core/src/app/components/inspector/inspector-panel.tsx`:
- Line 933: Update the submit function in the inspector panel to catch and
handle errors from the awaited onAdd call, providing the existing user feedback
for failed submissions. Keep both keyboard and button handlers invoking submit
so all submission paths use the same rejection handling, and avoid discarding a
potentially rejected promise with only void submit().
- Around line 691-694: The wrapper labels do not name the native color controls.
Move aria-label={label} from each color-swatch wrapper onto its nested input
type="color" in
packages/core/src/app/components/inspector/inspector-panel.tsx#L706 and
packages/core/src/app/components/style-panel/style-panel.tsx#L240; remove it
from the wrappers while preserving their visual styling.
---
Outside diff comments:
In `@packages/core/src/app/components/asset-view.tsx`:
- Around line 1384-1405: Update the effect around searchSvgl in
packages/core/src/app/components/asset-view.tsx:1384-1405 to avoid the
dependency suppression by tracking the locale-dependent
t.asset.toastSearchFailed value or a stable error code, while preserving the
existing search cancellation behavior. Update commitCreate and its listener in
packages/core/src/app/components/sidebar/sidebar.tsx:113-126 so changing t,
onCreate, or the folder icon state cannot leave the listener with stale
render-closure values; use appropriate dependencies or refs for these values.
---
Nitpick comments:
In `@packages/core/src/app/components/inspector/asset-picker-dialog.tsx`:
- Line 60: Narrow the jsx-a11y/no-noninteractive-element-interactions
suppression from the file-level directive to the `<section>` drop zone in the
asset picker dialog. Use Oxlint’s element- or next-line-scoped directive
immediately around that section, and remove the broad suppression so unrelated
`<Dialog>` controls remain linted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6ee65be-eed2-4303-a21b-e74e2dc98d49
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.changeset/oxlint-biome-split.md.oxlintrc.jsonAGENTS.mdCONTRIBUTING.mdREADME.mdapps/demo/slides/open-slide-launch/index.tsxapps/web/components/landing/hero-setup.tsxapps/web/proxy.tsbiome.jsonpackage.jsonpackages/core/src/app/components/asset-view.tsxpackages/core/src/app/components/inspector/asset-picker-dialog.tsxpackages/core/src/app/components/inspector/inspector-panel.tsxpackages/core/src/app/components/overview-grid.tsxpackages/core/src/app/components/sidebar/sidebar.tsxpackages/core/src/app/components/slide-canvas.tsxpackages/core/src/app/components/slide-preload-layer.tsxpackages/core/src/app/components/style-panel/style-panel.tsxpackages/core/src/app/lib/assets.tspackages/core/src/app/lib/inspector/use-comments.tspackages/core/src/app/lib/print-ready.test.tspackages/core/src/app/routes/home-shell.tsxpackages/core/src/app/routes/home.tsxpackages/core/src/app/routes/presenter.tsxpackages/core/src/app/routes/slide.tsxpackages/core/src/cli/build.tspackages/core/src/editing/edit-ops.tspackages/core/src/files/assets.tspackages/core/src/vite/design-plugin.test.tspackages/core/tsconfig.json
💤 Files with no reviewable changes (1)
- packages/core/tsconfig.json
| "files": ["**/slides/**", "apps/web/components/landing/**"], | ||
| "rules": { | ||
| "jsx-a11y/no-static-element-interactions": "off", | ||
| "jsx-a11y/no-noninteractive-element-interactions": "off", | ||
| "jsx-a11y/prefer-tag-over-role": "off" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== oxlintrc context =="
if [ -f .oxlintrc.json ]; then
nl -ba .oxlintrc.json | sed -n '1,120p'
fi
echo
echo "== matching files count =="
# Use git ls-files to inspect tracked matching paths, not filesystem only.
git ls-files | awk '
/\/slides\// {a++}
/^apps\/web\/components\/landing\// {b++}
END {print "slides_tree:", a+0; print "landing_tree:", b+0}
'
echo
echo "== specific rule occurrences in matching tracked files =="
python3 - <<'PY'
import subprocess
import pathlib,textwrap
files = subprocess.check_output(["git","ls-files"], text=True).splitlines()
slides=[]
landing=[]
for f in files:
if "/slides/" in f:
slides.append(f)
elif f.startswith("apps/web/components/landing/"):
landing.append(f)
patterns=[
'setAsDropZones',
'dropZone',
'isDropZone',
'asset',
'files',
'onDrop',
'onDragOver',
'onDragEnter',
'onClick',
'interactions',
]
for p in patterns:
hits=[]
for f in slides+landing:
try:
data=pathlib.Path(f).read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if p in data:
hits.append(f"{f}:{data.count(p)}")
if hits:
print(f"\n[{p}] count={len(hits)}")
for h in hits[:50]:
print(" " + h)
if len(hits)>50:
print(f" ... and {len(hits)-50} more")
PY
echo
echo "== direct usages outside matching trees =="
python3 - <<'PY'
import subprocess,pathlib
files = subprocess.check_output(["git","ls-files"], text=True).splitlines()
terms=['setAsDropZones','dropZone','isDropZone']
for term in terms:
hits=[]
for f in files:
if pathlib.PurePath(f).is_absolute():
continue
try:
data=pathlib.Path(f).read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if term in data:
hits.append(f)
print(f"{term}: {len(hits)}")
for h in hits[:80]:
print(" ", h)
if len(hits)>80:
print(f" ... and {len(hits)-80} more")
PYRepository: 1weiho/open-slide
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -u
echo "== oxlintrc context =="
if [ -f .oxlintrc.json ]; then
awk '{printf "%6d %s\n", NR, $0}' .oxlintrc.json | sed -n '1,120p'
fi
echo
echo "== matching tracked files count =="
git ls-files | awk '
/\/slides\// {a++}
/^apps\/web\/components\/landing\// {b++}
END {print "slides_tree:", a+0; print "landing_tree:", b+0}
'
echo
echo "== relevant symbol occurrences in matching tracked files =="
python3 - <<'PY'
import subprocess, pathlib
files = subprocess.check_output(["git","ls-files"], text=True).splitlines()
slides=[f for f in files if "/slides/" in f]
landing=[f for f in files if f.startswith("apps/web/components/landing/")]
patterns=[
'setAsDropZones',
'dropZone',
'isDropZone',
'drop z',
'onDrop',
'onDragOver',
'onDragEnter',
'setDropZones',
]
for p in patterns:
hits=[]
for f in slides+landing:
try:
data=pathlib.Path(f).read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if p in data:
hits.append((f, data.count(p)))
hits.sort(key=lambda x:x[1], reverse=True)
if hits:
print(f"\n[{p}] count={len(hits)}")
for f, n in hits[:40]:
print(f" {f}: {n}")
if len(hits)>40:
print(f" ... and {len(hits)-40} more")
PY
echo
echo "== targeted drop-zone symbol occurrences outside matching trees =="
python3 - <<'PY'
import subprocess, pathlib
files = subprocess.check_output(["git","ls-files"], text=True).splitlines()
terms=['setAsDropZones','dropZone','isDropZone','setDropZones']
matcher=lambda f: "/slides/" in f or f.startswith("apps/web/components/landing/")
for term in terms:
in_tree=[]
out_tree=[]
for f in files:
try:
data=pathlib.Path(f).read_text(encoding="utf-8", errors="ignore")
except Exception:
continue
if term in data:
(out_tree if not matcher(f) else in_tree).append(f)
print(f"{term}: in_tree={len(in_tree)}, out_tree={len(out_tree)}")
for f in out_tree[:80]:
print(" ", f)
PY
echo
echo "== rule occurrences outside .oxlintrc.json =="
if command -v rg >/dev/null 2>&1; then
rg -n 'no-static-element-interactions|no-noninteractive-element-interactions|prefer-tag-over-role|files.*slides|components/landing' -g '!node_modules' . | sed -n '1,160p'
else
grep -RInE 'no-static-element-interactions|no-noninteractive-element-interactions|prefer-tag-over-role|slide|landing' . | sed -n '1,160p'
fiRepository: 1weiho/open-slide
Length of output: 7228
Narrow the accessibility override to the actual drop-zone files.
These tree-level overrides affect every tracked file under both patterns (apps/web/components/landing/** includes navigation and other landing components). Disable these rules only where the drop-zone exception is actually needed, e.g. specific component paths or targeted file names, so new inaccessible controls elsewhere aren’t excluded from Oxlint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.oxlintrc.json around lines 41 - 46, Restrict the accessibility rule
overrides in the .oxlintrc configuration from the broad slides and landing
directory globs to the specific drop-zone component files that require them.
Preserve the three disabled rules for those targeted files while ensuring
navigation and other landing components remain subject to Oxlint accessibility
checks.
| pnpm lint # oxlint (JS/TS) + biome (CSS/JSON) | ||
| pnpm lint:js # oxlint only | ||
| pnpm lint:nonjs # biome non-JS lint only |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep CSS lint ownership documentation consistent with biome.json.
The final configuration disables Biome CSS linting, so these lines promise coverage that pnpm lint does not provide. Enable CSS linting or describe the actual JSON/non-JavaScript scope.
AGENTS.md#L27-L29: correct thebiome (CSS/JSON)and non-JavaScript wording.CONTRIBUTING.md#L56-L56: correct the CSS/JSON ownership description.README.md#L91-L91: correct the CSS/JSON ownership description.
📍 Affects 3 files
AGENTS.md#L27-L29(this comment)CONTRIBUTING.md#L56-L56README.md#L91-L91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 27 - 29, Update the lint ownership documentation to
match biome.json’s disabled CSS linting: in AGENTS.md lines 27-29,
CONTRIBUTING.md line 56, and README.md line 91, replace CSS/JSON and
non-JavaScript claims with the actual JSON/non-JavaScript scope while preserving
the existing command descriptions.
| ## Hard rules | ||
|
|
||
| - **Biome must pass before commit.** Run `pnpm check` (or `pnpm check:fix`). CI and the user's review both expect a clean tree. | ||
| - **Format and lint must pass before commit.** Run `pnpm check` (or `pnpm check:fix`). Oxlint owns JS/TS semantics; Biome owns formatting and non-JS lint. CI and the user's review both expect a clean tree. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not present check:fix as an alternative validation command.
check:fix runs fix commands but does not run typecheck or a final validation pass. Document it as a preparatory command, followed by pnpm check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 39, Update the validation guidance in AGENTS.md so pnpm
check:fix is described only as a preparatory formatting/lint-fix command,
followed by pnpm check for typechecking and final validation; do not present
check:fix as an alternative to pnpm check.
| "includes": ["**/*.{js,jsx,ts,tsx,mjs,cjs,mts,cts}", "**/*.d.ts"], | ||
| "linter": { | ||
| "rules": { | ||
| "a11y": { | ||
| "noSvgWithoutTitle": "off", | ||
| "noStaticElementInteractions": "off", | ||
| "useAriaPropsSupportedByRole": "off", | ||
| "useSemanticElements": "off" | ||
| }, | ||
| "performance": { | ||
| "noImgElement": "off" | ||
| }, | ||
| "complexity": { | ||
| "noUselessFragments": "off" | ||
| }, | ||
| "security": { | ||
| "noDangerouslySetInnerHtml": "off" | ||
| }, | ||
| "suspicious": { | ||
| "noArrayIndexKey": "off", | ||
| "noTemplateCurlyInString": "off", | ||
| "noShadowRestrictedNames": "off" | ||
| } | ||
| } | ||
| "enabled": false |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching biome/oxlint/config scripts:\n'
git ls-files | rg '(^|/)(biome\.json|oxlint|oxlint\.json|eslint\.config|package\.json|pnpm-lock\.yaml|biome\.jsonc|\.oxlintrc)' || true
printf '\nbiome.json (if present):\n'
if [ -f biome.json ]; then
nl -ba biome.json | sed -n '1,220p'
fi
printf '\nOxlint/config related files content snippets:\n'
for f in $(git ls-files | rg '(^|/)(oxlint|\.oxlintrc|eslint\.config|package\.json)$'); do
printf '\n--- %s ---\n' "$f"
nl -ba "$f" | sed -n '1,220p'
done
printf '\nSearch security/relevant rule names:\n'
rg -n "noDangerouslySetInnerHtml|noDangerouslySetInnerHtmlWithChildren|noGlobalEval|dangerouslySetInnerHTML|global.*eval|no.*eval|dangerous|oxlint|lint" -S . --glob '!node_modules' --glob '!dist' --glob '!build' | sed -n '1,240p'Repository: 1weiho/open-slide
Length of output: 524
🌐 Web query:
Biome noDangerouslySetInnerHtml noGlobalEval rules security dangerous HTML eval
💡 Result:
Biome provides several security-focused lint rules designed to prevent vulnerabilities related to arbitrary code execution and Cross-Site Scripting (XSS), including the following: noDangerouslySetInnerHtml: This rule (lint/security/noDangerouslySetInnerHtml) prevents the use of the dangerouslySetInnerHTML prop in React, which can expose users to XSS attacks by injecting unescaped HTML [1][2]. It is enabled by default and set to error severity [1]. noGlobalEval: This rule (lint/security/noGlobalEval) prohibits the use of global eval, which evaluates strings as JavaScript code [3]. This is considered a security risk because it allows for arbitrary code execution if the input is untrusted, and it also negatively impacts performance by preventing JavaScript engine optimizations [3]. Additional related security rules include: - noDangerouslySetInnerHtmlWithChildren: Reports when an element uses both children and dangerouslySetInnerHTML simultaneously [4][5]. - noScriptUrl: Disallows the use of javascript: URLs in HTML (e.g., in href attributes), as they act as a form of eval and can lead to XSS [6]. - noImpliedEval: A nursery rule (currently unstable) that flags eval-like methods such as using setTimeout, setInterval, or the Function constructor with string arguments [7]. These rules are part of Biome's focus on identifying patterns that lead to security vulnerabilities and performance issues, favoring safer alternatives like direct function references or standard JSX rendering [1][3][7].
Citations:
- 1: https://biomejs.dev/linter/rules/no-dangerously-set-inner-html/
- 2: https://github.com/biomejs/biome/blob/main/crates/biome_js_analyze/src/lint/security/no_dangerously_set_inner_html.rs
- 3: https://biomejs.dev/linter/rules/no-global-eval/
- 4: https://biomejs.dev/linter/javascript/rules/
- 5: https://biomejs.dev/linter/rules/no-dangerously-set-inner-html-with-children/
- 6: https://biomejs.dev/linter/rules/no-script-url/
- 7: https://biomejs.dev/linter/rules/no-implied-eval/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'biome.json excerpt:\n'
awk '{printf "%6d\t%s\n", NR, $0}' biome.json | sed -n '1,120p'
printf '\n.oxlintrc.json:\n'
awk '{printf "%6d\t%s\n", NR, $0}' .oxlintrc.json | sed -n '1,220p'
printf '\nRelevant rule/config mentions:\n'
for f in biometxt=".biome.json" || true; do
true
done
rg -n "noDangerouslySetInnerHtml|noDangerouslySetInnerHtmlWithChildren|noGlobalEval|noUndocumentedGlobals|dangerouslySetInnerHTML|dangerous|eval|allowComments|noScriptUrl|noImpliedEval|noDangerous|dangerous" -S .biome.json biome.json .oxlintrc.json package.json 2>/dev/null || true
printf '\nPackage scripts:\n'
python3 - <<'PY'
import json, pathlib
for p in ["package.json", ".oxlintrc.json", "biome.json"]:
if pathlib.Path(p).exists():
print(f"\n--- {p} ---")
data=json.loads(pathlib.Path(p).read_text())
if p=="package.json":
print(json.dumps({"scripts": data.get("scripts", {})}, indent=2)[:2000])
else:
print(json.dumps(data, indent=2)[:3000])
PYRepository: 1weiho/open-slide
Length of output: 3299
Preserve Biome’s JS/TS security checks or add equivalent Oxlint rules.
The JS/TS override disables Biome security rules such as lint/security/noDangerouslySetInnerHtml, lint/security/noDangerouslySetInnerHtmlWithChildren, and lint/security/noGlobalEval for all JavaScript/TypeScript files. The Oxlint config does not enable equivalent security rules, so this split removes the existing XSS/code-execution diagnostics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@biome.json` around lines 31 - 33, Update the JavaScript/TypeScript override
in biome.json so Biome’s security lint rules remain enabled, or configure
equivalent Oxlint security rules for the same file patterns. Ensure checks
covering dangerouslySetInnerHtml, dangerouslySetInnerHtmlWithChildren, and
noGlobalEval are preserved rather than disabled by the linter.enabled setting.
| if (!saving) void commit(); | ||
| }} | ||
| onKeyDown={(event) => { | ||
| if (event.nativeEvent.isComposing) return; | ||
| if (event.key === 'Enter') { | ||
| event.preventDefault(); | ||
| commit(); | ||
| void commit(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate files:"
git ls-files | rg 'packages/core/src/app/(components/asset-view\.tsx|routes/slide\.tsx)$' || true
echo
echo "Relevant asset-view lines:"
sed -n '1110,1200p' packages/core/src/app/components/asset-view.tsx
echo
echo "Relevant slide lines:"
sed -n '1085,1120p' packages/core/src/app/routes/slide.tsx
echo
echo "Search commit definitions/callback usages:"
rg -n "function .*commit|const .*commit|commit=\(|commit=\(\)|onBlur=|onKeyDown|Promise|reject|Error|Async" packages/core/src/app -g '*.tsx' -g '*.ts' | head -n 300Repository: 1weiho/open-slide
Length of output: 27297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "asset-view rename component definition:"
sed -n '1030,1105p' packages/core/src/app/components/asset-view.tsx
sed -n '1100,1135p' packages/core/src/app/components/asset-view.tsx
echo
echo "slide rename component definition:"
sed -n '1030,1105p' packages/core/src/app/routes/slide.tsx
sed -n '1100,1115p' packages/core/src/app/routes/slide.tsx
echo
echo "Component callers for RenameEditable:"
rg -n "<RenameEditable|RenameEditable<|onSubmit=\\(" packages/core/src/app -g '*.tsx' -g '*.ts' -C 3
echo
echo "Read-only behavioral probe: unhandled async rejection logged by Node when using void"
node - <<'JS'
let logged;
process.on('unhandledRejection', (reason, promise) => {
logged = String(reason);
});
async function reject() {
throw new Error('rename failed');
}
(async () => {
void reject();
await new Promise(resolve => setTimeout(resolve, 20)).then(() => {
console.log(logged);
});
})();
JSRepository: 1weiho/open-slide
Length of output: 5712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find RenameEditable declarations/usages by regex across tracked files:"
rg -n "function RenameEditable|const Rename|class Rename|type Rename|interface Rename|<RenameEditable|RenameEditable<|onSubmit=\(next|" packages/core/src -g '*.tsx' -g '*.ts' -C 4 || true
echo
echo "Surrounding RenameAsset usages:"
rg -n "RenameAsset<" packages/core/src/app -g '*.tsx' -g '*.ts' -C 8 || true
echo
echo "Exact rename call sites containing toast/error handling with await onSubmit or promise rejection:"
rg -n "await onSubmit|onSubmit\\\\(|toast|toast\\.error|\\.catch|unhandled" packages/core/src/app -g '*.tsx' -g '*.ts' -C 4 || true
echo
echo "Read-only behavioral probe: unhandled async rejection logged by Node when using void"
node - <<'JS'
let logged = null;
process.on('unhandledRejection', (reason, promise) => {
logged = String(reason);
});
(async () => {
void Promise.reject(new Error('rename failed'));
await new Promise(resolve => setTimeout(resolve, 20));
console.log(logged);
})();
JSRepository: 1weiho/open-slide
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "RenameAsset declarations/usages in app subdir only (limited):"
rg -n "function RenameAsset|<RenameAsset|onSubmit=\(" packages/core/src/app -g '*.tsx' -C 6 || true
echo
echo "Renamed asset toast/error call sites in app/assets.ts:"
sed -n '1,80p' packages/core/src/app/lib/assets.ts
rg -n "renameAsset|toast\.error|toastRename|rename\(" packages/core/src/app lib packages/core/src -g '*.tsx' -g '*.ts' -C 4 || true
echo
echo "Slide commit caller context around InlineTitleEditor if present:"
rg -n "InlineTitleEditor|renameSlide|toastSlide|toast.*rename|toast.*failed" packages/core/src/app/routes/slide.tsx -C 8 || true
echo
echo "Focused text matches around RenameEditable in all tracked files:"
rg -n "RenameEditable|onSubmit=\(\w+\)" packages/core/src -g '*.tsx' -g '*.ts' --glob '!node_modules' -C 4 | head -n 200 || trueRepository: 1weiho/open-slide
Length of output: 50373
Handle rename promise rejections at each commit boundary.
These handlers return promises to a commit() that is fired with void, so failures can surface as unhandled rejections. Catch/reject-log/retry in RenameAsset.commit() and InlineTitleEditor.commit() before setSaving(false).
📍 Affects 2 files
packages/core/src/app/components/asset-view.tsx#L1135-L1141(this comment)packages/core/src/app/components/asset-view.tsx#L1185-L1191packages/core/src/app/routes/slide.tsx#L1105-L1112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/app/components/asset-view.tsx` around lines 1135 - 1141,
Handle promise rejections in RenameAsset.commit() and InlineTitleEditor.commit()
before setSaving(false), using the existing error logging or retry behavior as
appropriate. Apply the fix to both commit-triggering handlers in
packages/core/src/app/components/asset-view.tsx (lines 1135-1141 and 1185-1191)
and the corresponding handler in packages/core/src/app/routes/slide.tsx (lines
1105-1112); each site must avoid unhandled rejections from void commit() calls.
| if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) { | ||
| e.preventDefault(); | ||
| submit(); | ||
| void submit(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle rejected submissions before discarding the promise.
void submit() only satisfies the no-floating-promises rule. submit awaits onAdd, and the supplied useComments.add throws on failed POST requests, so keyboard submission can produce an unhandled rejection with no user feedback. Handle the error inside submit so both keyboard and button submissions are covered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/app/components/inspector/inspector-panel.tsx` at line 933,
Update the submit function in the inspector panel to catch and handle errors
from the awaited onAdd call, providing the existing user feedback for failed
submissions. Keep both keyboard and button handlers invoking submit so all
submission paths use the same rejection handling, and avoid discarding a
potentially rejected promise with only void submit().
…abbit - Catch promise rejections in RenameAsset.commit, InlineTitleEditor.commit, and inspector comment submit to avoid unhandled rejections. - Move aria-label from color-swatch wrapper <label> onto the native <input type="color"> in inspector-panel and style-panel. - Narrow the jsx-a11y/no-noninteractive-element-interactions suppression in asset-picker-dialog from file-level to the drop-zone <section> only. - Tighten the oxlint changeset description.
|
Addressed CodeRabbit review — pushed fixes for the actionable items: Fixed:
Skipped (with rationale):
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/app/routes/slide.tsx`:
- Around line 1079-1081: Update the catch block in InlineTitleEditor to display
a localized error toast when renameSlide fails, matching the existing asset
rename error-toast pattern in asset-view.tsx. Preserve the current
setValue(title) and setEditing(false) cleanup behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e47992b-80f1-4aa7-8d64-a10439f43342
📒 Files selected for processing (6)
.changeset/oxlint-biome-split.mdpackages/core/src/app/components/asset-view.tsxpackages/core/src/app/components/inspector/asset-picker-dialog.tsxpackages/core/src/app/components/inspector/inspector-panel.tsxpackages/core/src/app/components/style-panel/style-panel.tsxpackages/core/src/app/routes/slide.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/app/components/inspector/asset-picker-dialog.tsx
- packages/core/src/app/components/inspector/inspector-panel.tsx
2 participants
Issue
Closes / implements #321 (parent roadmap #318).
This branch is independent — it branches directly from
mainand contains only the Oxlint adoption changes. It does not depend on #326, #369, or #370; the lint stack is orthogonal to the runtime/React/Vite migrations.Exact scope
Versions before → after
oxlint1.74.0oxlint-tsgolint0.24.0@biomejs/biome2.4.122.4.12(unchanged)Why the old state was a problem
Biome owned JS/TS lint, formatting, and CSS/JSON together. That duplicated parser generations vs the Vite 8 / Oxc / Rolldown stack and limited type-aware / multi-file semantic checks (floating promises, import cycles).
Why these tools/versions were chosen
1.74.0+oxlint-tsgolint0.24.0for type-aware rulestypescript/no-floating-promisesandimport/no-cycleBreaking changes reviewed
pnpm lint/pnpm checksemantics changed (documented in README / CONTRIBUTING / AGENTS / CLAUDE)packages/core/tsconfig.json: removedbaseUrlso type-aware lint works (tsgolint rejectsbaseUrl; see Crash when running tsgolint:Option 'baseUrl' has been removed.oxc-project/tsgolint#351).pathsremain;pnpm typecheckstill passesSource changes required
||expression, useless spreads, floating promises (void), hook deps, a11y labels, unbound method on presenter channel, Vitest mock typing / conditional expectjsx-a11y/prefer-tag-over-roleleft off (intentional ARIA/rolepatterns; BiomeuseSemanticElementswas already selectively off for slides/landing)Tests added or changed
print-ready.test.ts: typedvi.fn<() => Promise<void>>()design-plugin.test.ts: replace conditional expect withtoEqualRule mapping table
linter.rules.recommended(correctness/suspicious/complexity/…)categories.correctness: error+ default pluginsuseExhaustiveDependencies)reactplugin +react-hooks/exhaustive-depsnoStaticElementInteractions, labels, …)jsx-a11ypluginuseSemanticElements/ prefer semantic tagsjsx-a11y/prefer-tag-over-rolerolepatterns**/slides/**,apps/web/components/landing/**assist.organizeImportstypescript/no-floating-promisesvoidimport/no-cyclevitest/*correctnessassertFunctionNamesincludes helperspackages/core/src/app/components/ui/**packages/cli/template/**No previous JS/TS coverage was dropped silently; gaps above are classified.
Benchmark results
Same machine, Node 24.18.0, pnpm 10.17.0. One warm-up + 10 measured runs (median / p95). Times for wrapped
pnpm/pnpm execinclude package-manager startup.pnpm lintuseOptionalChain)Type-aware on vs off (Oxlint only): median 1.168 s vs 0.598 s (~2×). Decision: keep type-aware on in
lint:jsfor floating-promise + cycle coverage;lint:js:no-type-awareavailable for local comparison.Manual validation results
On Node 24.18.0 with pnpm 10.17.0 (current
maintoolchain):Known limitations
jsx-a11y/prefer-tag-over-roleintentionally offProhibited workarounds
Confirmed not used:
--force,--legacy-peer-deps, dependency overrides/resolutions, patch-package, vendored forks, blanket TS/lint suppressions, prerelease packages, silent snapshot updates, unrelated features/UI churn.Summary by CodeRabbit
aria-labels for slide setup-mode selectors, color inputs, asset selection/upload controls, and drop zones.