Skip to content

chore: update ESLint configuration and improve image handling#195

Open
tnkshuuhei wants to merge 1 commit intofeat/semantic-scholarfrom
fix/lint
Open

chore: update ESLint configuration and improve image handling#195
tnkshuuhei wants to merge 1 commit intofeat/semantic-scholarfrom
fix/lint

Conversation

@tnkshuuhei
Copy link
Member

  • Added @typescript-eslint/eslint-plugin to ESLint configuration for better TypeScript support.
  • Disabled the default no-unused-vars rule and replaced it with @typescript-eslint/no-unused-vars for improved linting of unused variables.
  • Updated ImagePreview component to use Next.js Image component for optimized image loading.
  • Minor refactor in step-process-dialog to simplify step mapping.
  • Added eslint-disable comment for native usage in OG image generation.

- Added @typescript-eslint/eslint-plugin to ESLint configuration for better TypeScript support.
- Disabled the default no-unused-vars rule and replaced it with @typescript-eslint/no-unused-vars for improved linting of unused variables.
- Updated ImagePreview component to use Next.js Image component for optimized image loading.
- Minor refactor in step-process-dialog to simplify step mapping.
- Added eslint-disable comment for native <img> usage in OG image generation.
@vercel
Copy link

vercel bot commented Feb 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
muse Building Building Preview, Comment Feb 26, 2026 7:04am

@claude
Copy link

claude bot commented Feb 26, 2026

PR Review: chore: update ESLint configuration and improve image handling

Overall this is a clean, well-scoped PR. The intent behind each change is clear. A few things worth addressing before merging:


Bug / Missing Dependency

@typescript-eslint/eslint-plugin is not in package.json.

eslint.config.mjs now imports tseslintPlugin from @typescript-eslint/eslint-plugin, but only @typescript-eslint/parser is listed as an explicit dependency. This likely works right now because the plugin is pulled in as a transitive dependency of something else, but that is fragile — a package update could break it silently.

Run bun add -D @typescript-eslint/eslint-plugin to pin it explicitly alongside the parser.


ESLint Config Changes

The no-unused-vars to @typescript-eslint/no-unused-vars migration is correct and necessary — the native rule produces false positives on type-only imports and TypeScript-specific patterns. The ignore patterns (^_) are a well-established convention. Good change.

One small note: the plugin is configured without parserOptions.project, so type-aware rules are unavailable. That is fine for now since no-unused-vars does not need type information, but worth knowing if type-aware rules are added later.

Consider promoting @typescript-eslint/no-unused-vars from "warn" to "error" if the goal is to enforce it strictly (team preference).


ImagePreview — next/image for Data URLs

This satisfies the @next/next/no-img-element lint rule, which is the apparent motivation. A few observations:

  • unoptimized is correct and necessary — Next.js Image optimization does not apply to data URLs, and omitting it would cause a runtime warning/error.
  • The width/height + CSS override pattern (dimensions set to 1200x630 then overridden to auto via style) is a valid workaround for responsive display, but the dimensions are hardcoded and happen to match the OG image aspect ratio. If the canvas output size ever changes, this could produce subtle layout issues (wrong intrinsic aspect ratio). Consider sourcing these from a shared constant, or using actual image dimensions from CanvasImageResult if available.
  • Since this is a data URL displayed in-app (not a remote image), there is no issue with next.config.ts image domain allow-lists — data URLs bypass the image optimizer entirely.

OG Image Route — eslint-disable Comment

This is exactly the right approach. Satori (the renderer behind ImageResponse) does not support the Next.js <Image> component — native <img> is required. The comment explains the reason clearly.


Step Process Dialog — Unused index Removal

Clean and correct. index was genuinely unused and the key is already step.id.


Summary

Area Status
Missing @typescript-eslint/eslint-plugin dep Needs fix
ESLint rule migration (no-unused-vars) Correct
next/image with unoptimized for data URLs Works, minor concern on hardcoded dimensions
OG image eslint-disable comment Correct
Unused index removal Good cleanup

The only blocking issue is the missing explicit package dependency. Everything else is good to go after that is addressed.

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.

1 participant