Skip to content

Conversation

@ItzNotABug
Copy link
Member

@ItzNotABug ItzNotABug commented Nov 6, 2025

What does this PR do?

Fixes - #2564

Test Plan

Manual.

selection.bug.mp4

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes.

Summary by CodeRabbit

  • Chores

    • Upgraded package manager from 10.15.1 to 10.20.0 for enhanced stability
    • Improved development workflow by adding caching to the formatting utility
  • Bug Fixes

    • Fixed data binding issues in spreadsheet table display
    • Enhanced automatic UI refresh when spreadsheet data is updated

@ItzNotABug ItzNotABug self-assigned this Nov 6, 2025
@appwrite
Copy link

appwrite bot commented Nov 6, 2025

Console

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Cursor pagination performs better than offset pagination when loading further pages.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

The pull request updates package dependencies and build configuration, along with reactive store handling in a Svelte component. The package.json adds caching to the Prettier formatting script and upgrades the pnpm package manager version from 10.15.1 to 10.20.0. In the spreadsheet component, a reactive condition is updated from checking a store wrapper to dereferencing the store value directly, and a UI reset mechanism is added that updates a render key using a timestamp hash when data changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify the store dereferencing change ($rows) correctly accesses the store value in the reactive block context
  • Confirm the timestamp hashing logic for spreadsheetRenderKey produces consistent hashes and effectively triggers re-renders
  • Validate that pnpm 10.20.0 is a compatible upgrade with no breaking changes for the current toolchain
  • Ensure the Prettier --cache flag does not introduce unexpected build behavior or cache invalidation issues

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing a selection and deletion bug on the spreadsheet component, which aligns with the primary code change in spreadsheet.svelte.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-dat-865

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28d90a2 and f6da8de.

📒 Files selected for processing (2)
  • package.json (2 hunks)
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
⏰ 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: build
  • GitHub Check: e2e
🔇 Additional comments (3)
package.json (2)

15-15: LGTM: Prettier caching improves performance.

Adding the --cache flag to the format script will improve performance by caching formatting results for unchanged files.


98-98: Verify pnpm onlyBuiltDependencies configuration is accurate.

The upgrade from pnpm 10.15.1 to 10.20.0 introduces breaking changes, most notably that dependency lifecycle scripts no longer run by default in pnpm 10. Your project is already configured with onlyBuiltDependencies (line 91), mitigating this risk.

However, the configured list includes @parcel/watcher, @sentry/cli, and esbuild which are not in direct dependencies—likely transitive or stale entries. Before upgrading, verify:

  • Whether these transitive dependencies actually require lifecycle scripts to run during installation
  • Whether @sentry/sveltekit (which is in your dependencies) should replace @sentry/cli in the list
  • That the final configuration matches your actual build requirements

Run pnpm install after the upgrade and verify builds/tests pass, as peer dependency resolution behavior and other minor changes may affect your dependency tree.

src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)

107-113: LGTM: Fixes reactive condition and adds UI reset for data changes.

The change from if (rows) to if ($rows) correctly checks the store value instead of the store wrapper. The previous condition would always be truthy once the store was created, making it ineffective. The new condition properly verifies that data exists before resetting pagination.

The UI reset mechanism (line 112) forces a re-render when data changes by updating the render key, which should resolve the selection and deletion bug mentioned in the PR objectives by clearing stale UI state.


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.

❤️ Share

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

@ItzNotABug ItzNotABug merged commit 0068c3c into main Nov 6, 2025
4 checks passed
@ItzNotABug ItzNotABug deleted the fix-dat-865 branch November 6, 2025 06:44
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.

3 participants