♻️ refactor(erd-core): add Hash schema and integrate URL handling functions #3702
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds URL utilities for table link hrefs and column element IDs, introduces a hash schema and type for column element IDs, re-exports the new utilities and schema barrels, updates two components to use the shared utilities, and adds tests for the URL helper and hash schema. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component (ColumnsItem / TableOptions)
participant Utils as URL utils
participant Browser as window.location
UI->>Utils: getTableColumnElementId(tableId, columnName)
Note right of Utils #D0F0C0: pure ID string generation
Utils-->>UI: "#<table>__columns__<column>"
UI->>Utils: getTableLinkHref(activeTableName)
Utils->>Browser: read window.location.search
Note right of Utils #F0E68C: preserve existing params, set active
Utils-->>UI: "?...&active=<table>"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
Updates to Preview Branch (refactor/table-column-url-functions) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
| import type { InferOutput } from 'valibot' | ||
| import type { hashSchema } from './schemas' | ||
|
|
||
| export type Hash = InferOutput<typeof hashSchema> |
There was a problem hiding this comment.
This Hash type is same with ${string}__columns__${string}. It will extend its definition when we allow other hash values by updating hashSchema.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/packages/erd-core/src/schemas/hash/schemas.ts (1)
3-9: Consider validating that both parts are non-empty.The current implementation allows strings like
"__columns__id"or"users__columns__"(with empty table or column names) to pass validation. If both the table name and column ID should be non-empty, add additional checks.If stricter validation is desired, apply this diff:
export const hashSchema = custom<`${string}__columns__${string}`>( (input): input is `${string}__columns__${string}` => { if (typeof input !== 'string') return false const parts = input.split('__columns__') - return parts.length === 2 + return parts.length === 2 && parts[0] !== '' && parts[1] !== '' }, )frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts (1)
4-16: Add test cleanup to prevent pollution.The tests mutate
window.location.searchbut don't reset it afterward. This can cause test pollution if tests run in a specific order or interfere with other tests.Add cleanup using
beforeEachandafterEach:+import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { getTableLinkHref } from './getTableLinkHref' +let originalSearch: string + +beforeEach(() => { + originalSearch = window.location.search +}) + +afterEach(() => { + window.location.search = originalSearch +}) + it('should return the "active" query parameter with the table name', () => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx(2 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx(1 hunks)frontend/packages/erd-core/src/features/erd/utils/index.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/index.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/index.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/schemas.test.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/schemas.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/types.ts(1 hunks)frontend/packages/erd-core/src/schemas/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/packages/erd-core/src/features/erd/utils/url/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.tsfrontend/packages/erd-core/src/schemas/index.tsfrontend/packages/erd-core/src/schemas/hash/types.tsfrontend/packages/erd-core/src/schemas/hash/schemas.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/schemas/hash/schemas.test.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.tsfrontend/packages/erd-core/src/schemas/hash/index.tsfrontend/packages/erd-core/src/features/erd/utils/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/packages/erd-core/src/features/erd/utils/url/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.tsfrontend/packages/erd-core/src/schemas/index.tsfrontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsxfrontend/packages/erd-core/src/schemas/hash/types.tsfrontend/packages/erd-core/src/schemas/hash/schemas.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/schemas/hash/schemas.test.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.tsfrontend/packages/erd-core/src/schemas/hash/index.tsfrontend/packages/erd-core/src/features/erd/utils/index.ts
frontend/packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Shared libraries and tools live under frontend/packages
Files:
frontend/packages/erd-core/src/features/erd/utils/url/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.tsfrontend/packages/erd-core/src/schemas/index.tsfrontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsxfrontend/packages/erd-core/src/schemas/hash/types.tsfrontend/packages/erd-core/src/schemas/hash/schemas.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/schemas/hash/schemas.test.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.tsfrontend/packages/erd-core/src/schemas/hash/index.tsfrontend/packages/erd-core/src/features/erd/utils/index.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/packages/erd-core/src/features/erd/utils/url/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.tsfrontend/packages/erd-core/src/schemas/index.tsfrontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsxfrontend/packages/erd-core/src/schemas/hash/types.tsfrontend/packages/erd-core/src/schemas/hash/schemas.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/schemas/hash/schemas.test.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.tsfrontend/packages/erd-core/src/schemas/hash/index.tsfrontend/packages/erd-core/src/features/erd/utils/index.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
Prefix event handler functions with “handle” (e.g., handleClick)
Files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with filenames ending in .test.ts or .test.tsx colocated near source
Files:
frontend/packages/erd-core/src/schemas/hash/schemas.test.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts
🧠 Learnings (2)
📚 Learning: 2025-08-23T03:36:21.590Z
Learnt from: tnyo43
PR: liam-hq/liam#3129
File: frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx:11-15
Timestamp: 2025-08-23T03:36:21.590Z
Learning: The CommandPalette and its child components (including CommandPaletteContent and TableOptions) in the ERD renderer are only rendered on the client side, so SSR safety checks for window object access are not necessary in this context.
Applied to files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx
📚 Learning: 2025-07-30T05:52:56.270Z
Learnt from: hoshinotsuyoshi
PR: liam-hq/liam#2771
File: frontend/internal-packages/schema-bench/src/cli/executeLiamDb.ts:22-22
Timestamp: 2025-07-30T05:52:56.270Z
Learning: The schema-bench package (frontend/internal-packages/schema-bench) has been converted from ESM to CommonJS mode by removing "type": "module" from package.json, making __dirname available and correct to use in TypeScript files within this package.
Applied to files:
frontend/packages/erd-core/src/schemas/hash/index.ts
🧬 Code graph analysis (4)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts (1)
getTableColumnElementId(3-6)
frontend/packages/erd-core/src/schemas/hash/types.ts (1)
frontend/packages/erd-core/src/schemas/hash/schemas.ts (1)
hashSchema(3-9)
frontend/packages/erd-core/src/schemas/hash/schemas.test.ts (1)
frontend/packages/erd-core/src/schemas/hash/schemas.ts (1)
hashSchema(3-9)
frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts (1)
getTableLinkHref(3-7)
⏰ 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). (4)
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: security-review
- GitHub Check: Supabase Preview
🔇 Additional comments (8)
frontend/packages/erd-core/src/features/erd/utils/url/index.ts (1)
1-2: LGTM!Clean barrel file following standard re-export patterns.
frontend/packages/erd-core/src/features/erd/utils/index.ts (1)
8-8: LGTM!Consistent with existing export pattern in the file.
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (1)
10-10: LGTM!Good refactoring to extract the element ID generation to a shared utility. The usage is consistent with the utility's intended purpose.
Also applies to: 33-33
frontend/packages/erd-core/src/schemas/hash/types.ts (1)
1-4: LGTM!Standard Valibot pattern for deriving TypeScript types from runtime schemas.
frontend/packages/erd-core/src/schemas/index.ts (1)
1-1: LGTM!Consistent with existing export pattern in the schemas index.
frontend/packages/erd-core/src/schemas/hash/index.ts (1)
1-2: LGTM!Clean barrel file following standard re-export patterns for the hash module.
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx (1)
6-6: LGTM!Good refactoring to extract the link href generation to a shared utility. The usage at lines 42 and 62 is consistent and maintains the same functionality as the previous local implementation.
Also applies to: 42-42, 62-62
frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts (1)
3-7: Implementation is correct for browser environments.The function correctly constructs the query string using
URLSearchParamsand preserves existing parameters. The use ofsatisfies QueryParamprovides type safety at compile time.Note: Direct access to
window.location.searchmakes the function harder to test (tests must mutate the global window object) and not SSR-compatible. If testability or SSR support becomes a concern, consider acceptingwindow.location.searchas a parameter with a default value.
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts
Show resolved
Hide resolved
frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts
Outdated
Show resolved
Hide resolved
tnyo43
force-pushed
the
refactor/table-column-url-functions
branch
from
65f7ec1 to
8b2c946
Compare
tnyo43
requested review from
junkisai,
MH4GF,
NoritakaIkeda and
sasamuku
and removed request for
a team
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts (1)
3-7: Consider parameterizing the search source for better testability.The function directly accesses
window.location.search, which couples it tightly to the browser environment. This makes unit testing more difficult and prevents usage in server-side rendering contexts.Consider refactoring to accept the search string as an optional parameter:
-export const getTableLinkHref = (activeTableName: string) => { - const searchParams = new URLSearchParams(window.location.search) +export const getTableLinkHref = ( + activeTableName: string, + searchString = window.location.search +) => { + const searchParams = new URLSearchParams(searchString) searchParams.set('active' satisfies QueryParam, activeTableName) return `?${searchParams.toString()}` }This change would enable easier unit testing by allowing you to pass test query strings while maintaining the current behavior when called without the second parameter.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx(2 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx(1 hunks)frontend/packages/erd-core/src/features/erd/utils/index.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/index.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/index.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/schemas.test.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/schemas.ts(1 hunks)frontend/packages/erd-core/src/schemas/hash/types.ts(1 hunks)frontend/packages/erd-core/src/schemas/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/packages/erd-core/src/features/erd/utils/url/index.ts
- frontend/packages/erd-core/src/schemas/index.ts
- frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.test.ts
- frontend/packages/erd-core/src/schemas/hash/index.ts
- frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts
- frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx
- frontend/packages/erd-core/src/schemas/hash/schemas.ts
- frontend/packages/erd-core/src/schemas/hash/types.ts
- frontend/packages/erd-core/src/schemas/hash/schemas.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/packages/erd-core/src/features/erd/utils/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/packages/erd-core/src/features/erd/utils/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx
frontend/packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Shared libraries and tools live under frontend/packages
Files:
frontend/packages/erd-core/src/features/erd/utils/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/packages/erd-core/src/features/erd/utils/index.tsfrontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.tsfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
Prefix event handler functions with “handle” (e.g., handleClick)
Files:
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx
🧬 Code graph analysis (1)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts (1)
getTableColumnElementId(3-6)
⏰ 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). (5)
- GitHub Check: Supabase Preview
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: Supabase Preview
🔇 Additional comments (2)
frontend/packages/erd-core/src/features/erd/utils/index.ts (1)
8-8: LGTM!The re-export correctly exposes the new URL utilities as part of the public API, aligning with the PR objective to consolidate URL handling functions.
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (1)
10-10: LGTM!The refactor successfully consolidates the duplicate
columnElementIdhelper by using the sharedgetTableColumnElementIdutility, which aligns with the PR objective to integrate similar implementations across components.Also applies to: 33-33
Issue
- resolve:Why is this change needed?
We have implement some similar URL handling function in different components. This PR will integrate them into util directory. Also, it introduce
HashSchemato give better types to the functions.Summary by CodeRabbit