Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Nov 10, 2025

No description provided.

@vercel
Copy link

vercel bot commented Nov 10, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-inspector Ready Ready Preview Comment Nov 10, 2025 8:28pm
rivetkit-serverless Ready Ready Preview Comment Nov 10, 2025 8:28pm
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 10, 2025 8:28pm
rivet-site Ignored Ignored Preview Nov 10, 2025 8:28pm

Copy link
Contributor Author

jog1t commented Nov 10, 2025


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link

claude bot commented Nov 10, 2025

Code Review - PR #3436: Enable Biome for Frontend

Overview

This PR enables Biome linting/formatting for the frontend codebase, applying automated formatting across 27 files. The changes are primarily cosmetic improvements to code style and consistency.

Positive Observations ✅

  1. Consistent Import Ordering: Imports are now properly organized with type imports using type keyword (e.g., type QueryKey, type IconProp), which improves tree-shaking and makes type-only imports explicit.

  2. Better Code Formatting: Multi-line formatting has been improved for better readability:

    • Function parameters are properly wrapped
    • Object destructuring is consistently formatted
    • JSX attributes are properly aligned
  3. Configuration Updates:

    • biome.json properly includes frontend TypeScript/TSX files
    • tsconfig.json updated to support the new linting setup
  4. No Breaking Changes: All changes are formatting-only with no logic modifications, reducing risk of introducing bugs.

Issues & Concerns 🔍

1. Missing Type Import (Minor Issue)

Location: frontend/src/app/data-providers/engine-data-provider.tsx:8

UseQueryOptions,  // Should be: type UseQueryOptions

This import is used as a type but doesn't have the type keyword prefix, which is inconsistent with the other imports in the same file and the rest of the PR.

2. useEffect Dependency Array Warning (Potential Bug)

Location: frontend/src/components/hooks/use-async-memo.tsx:30

useEffect(() => {
  // ...
}, deps);

The deps parameter is passed directly to useEffect, but ESLint/React hooks rules typically expect a specific dependency array, not a spread parameter. While this works, it bypasses React's exhaustive-deps checking. Consider if this is intentional or if there's a better pattern.

3. Biome Configuration Issue (Configuration)

Location: biome.json:11-13

"frontend/**/*.tsx",
"frontend/**/*.ts",
"!frontend/packages",

The inclusion pattern seems redundant - the global patterns **/*.ts and **/*.json (lines 5-7) should already cover frontend files. The frontend-specific patterns might be unnecessary unless there's a specific reason to handle them separately. This could lead to confusion about which files are actually being linted.

4. Missing ESLint Rule for Import Sorting

The PR sorts imports but doesn't show any Biome configuration for import sorting rules. Verify that Biome is configured to enforce this automatically, otherwise the formatting might diverge in future changes.

Recommendations 📋

  1. Fix Type Import: Update line 8 in engine-data-provider.tsx to use type UseQueryOptions for consistency.

  2. Verify useEffect Pattern: Confirm that the useAsyncMemo hook's approach to dependency arrays is intentional and safe. Consider adding a comment explaining why exhaustive-deps is being bypassed.

  3. Simplify Biome Config: Consider removing the redundant frontend-specific patterns in biome.json unless there's a specific reason they're needed.

  4. Test Coverage: Since this is a style-only change, ensure that:

    • All existing tests still pass
    • The build succeeds without errors
    • No new linting warnings are introduced
  5. Documentation: Consider adding a note to the project's development docs about Biome being enabled for the frontend, including how to run it locally.

Security Considerations 🔒

No security concerns identified. All changes are formatting-only.

Performance Considerations ⚡

No performance impact. The changes are purely cosmetic.

Final Assessment

This is a good PR that improves code consistency and maintainability. The changes are low-risk and align with modern TypeScript/React best practices. The main issues are minor and should be quick fixes:

  • One missing type keyword for consistency
  • Potential configuration cleanup in biome.json

Recommendation: Approve with minor changes requested.


Note: This review was generated using the repository's coding standards from CLAUDE.md.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 10, 2025

More templates

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/actor@3436

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3436

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/core@3436

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3436

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3436

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3436

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3436

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3436

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3436

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3436

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3436

commit: bf9ca23

@jog1t jog1t marked this pull request as ready for review November 10, 2025 21:09
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 10, 2025

Graphite Automations

"Test" took an action on this PR • (11/10/25)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

Base automatically changed from 11-10-fix_fe_persist_ephemeral_filters_to_local_stroge to 11-10-fix_fe_make_serverfull_serverless_switcher_working_again November 10, 2025 21:12
@jog1t jog1t merged commit 34f4065 into 11-10-fix_fe_make_serverfull_serverless_switcher_working_again Nov 10, 2025
14 of 15 checks passed
@jog1t jog1t deleted the 11-10-style_fe_enable_biome_for_frontend branch November 10, 2025 21:18
jog1t added a commit that referenced this pull request Nov 10, 2025
* fix(fe): make serverfull/serverless switcher working again

* fix(fe): persist ephemeral filters to local stroge (#3435)

* style(fe): enable biome for frontend (#3436)

* fix(fe): persist ephemeral filters to local stroge

* style(fe): enable biome for frontend

* feat(fe): add crash-loop status to actors (#3437)

* fix(fe): persist ephemeral filters to local stroge

* style(fe): enable biome for frontend

* feat(fe): add crash-loop status to actors
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.

2 participants