Feat DocumentsDB
#2825
Feat DocumentsDB
#2825
Conversation
update: move things around.
update: db page to svelte5.
optimize: imports, singular sources of truth.
fix: height issues on mobile.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/customId.svelte (1)
29-71:⚠️ Potential issue | 🟠 MajorAvoid auto‑sync loops and ensure generated IDs start with alphanumeric.
toIdFormat()can yield""or a leading_/.. WithsyncFromtruthy this can bounceidbetween""andnull, and it can prefill invalid IDs. Guard empty results and strip leading non‑alphanumerics.🔧 Suggested fix
function toIdFormat(str: string): string { return str .toLowerCase() .replace(/[^a-z0-9\-_. ]+/g, '') .replace(/ /g, '_') - .replace(/^-+/, '') + .replace(/^[^a-z0-9]+/, '') .replace(/\.+$/, '') .replace(/_{2,}/g, '_') .slice(0, 36); // max length } $effect(() => { if (syncFrom && !touchedId) { const newId = toIdFormat(syncFrom); - if (id !== newId) { - id = newId; - } + if (!newId) { + if (id !== null) id = null; + return; + } + if (id !== newId) id = newId; } });
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/create/+page.svelte:
- Around line 214-222: The Cancel button currently calls
resetCreateDatabaseStore() immediately which clears the form before the user
confirms; change the Cancel Button's click handler to only set showExitModal =
true (remove the resetCreateDatabaseStore() call) so that form reset only
happens in the onExit handler (the existing onExit function should remain
responsible for calling resetCreateDatabaseStore()) and the modal dismissal
preserves user input.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte:
- Line 152: selectedPolicyGroup is assigned null but declared as string; update
its type annotation (the declaration of selectedPolicyGroup) to allow null
(e.g., string | null) so the assignment at the top (selectedPolicyGroup = null)
and any subsequent null checks are type-safe; locate the let/const declaration
named selectedPolicyGroup in the Svelte script block and change its type
accordingly.
- Around line 140-142: getPolicyById can return undefined when IDs are
randomized, so add null guards wherever its result is used: before calling
markPolicyChecked and before accessing .id or .label on the returned policy.
Specifically, update usages of getPolicyById(...) that are passed into
markPolicyChecked and any code that reads .id/.label to first check the result
(e.g., const policy = getPolicyById(id); if (!policy) return/skip/choose a
sensible default) or use optional chaining with a clear fallback value; keep
references to getPolicyById, markPolicyChecked and the presetPolicies lookup so
the changes are localized to those call sites.
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/create/+page.svelte (2)
39-40: Suspicious type assertions for database type.Line 39's
?? (null as DatabaseType)is redundant sincesearchParams.get()already returnsstring | null. CastingnulltoDatabaseTypeis a type lie that undermines type safety.♻️ Proposed fix
- const typeFromParams = page.url.searchParams.get('type') ?? (null as DatabaseType); - let type = $state(typeFromParams ?? 'tablesdb') as DatabaseType; + const typeFromParams = page.url.searchParams.get('type') as DatabaseType | null; + let type = $state<DatabaseType>(typeFromParams ?? 'tablesdb');
102-103: The function returns void—the assignment suggestion is not applicable.
cronExpression()is intentionally designed for side effects and returns void. The first suggested fix won't work. If clarity is desired, consider adding a brief comment:const totalPoliciesPromise = totalPolicies.map((policy) => { + // Mutates policy.schedule based on frequency configuration cronExpression(policy);src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte (1)
32-32: Consider propagatingdisabledto all interactive controls.If
disabledis meant to freeze edits during submit, it’s currently only applied to the daily InputSwitch. You may want to pass it through to the preset checkboxes/Card.Selector and action buttons for consistent lockout.Also applies to: 271-271
src/routes/(console)/project-[region]-[project]/databases/create/+page.svelte
Outdated
Show resolved
Hide resolved
...onsole)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
Show resolved
Hide resolved
...onsole)/project-[region]-[project]/databases/database-[database]/backups/createPolicy.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
552-581:⚠️ Potential issue | 🟠 MajorSet
databaseColumnSheetOptions.columnfor header actions.Update/delete flows rely on this store (the delete modal reads
.key). If it’s stale or null, the sheet can open with the wrong column or throw. Set it once when handling header actions.🐛 Proposed fix
if (type === 'header') { if (!columnId || !column) { return; } + $databaseColumnSheetOptions.column = column; if (action === 'update') { $databaseColumnSheetOptions.show = true; $databaseColumnSheetOptions.isEdit = true; $databaseColumnSheetOptions.title = 'Update column'; } if (action === 'column-left' || action === 'column-right') { const neighbour = columnId; const to = action === 'column-left' ? 'left' : 'right'; - $databaseColumnSheetOptions.column = column; $databaseColumnSheetOptions.direction = { neighbour: columnId, to: action === 'column-left' ? 'left' : 'right' };
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte:
- Line 6: The import using the non-approved alias "$database" (importing
SideSheet) violates the guideline limiting imports to $lib, $routes or $themes;
change the import of SideSheet to use one of the approved aliases (for example
import SideSheet from an equivalent module under $routes or $lib — e.g. move or
re-export the module under $lib or $routes and update the import for SideSheet
accordingly) so the code no longer references "$database" and continues to
resolve the same SideSheet symbol.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte (4)
46-46: Consider using a plainSetinstead ofSvelteSetfor the element cache.
cachedElementsis only used internally to track observed DOM elements for the ResizeObserver and is never bound to the template. Since reactivity isn't needed here, a plainSet<Element>would be more appropriate and slightly more efficient.♻️ Proposed fix
- import { SvelteSet } from 'svelte/reactivity';- let cachedElements = new SvelteSet<Element>(); + let cachedElements = new Set<Element>();
160-161: Consider using Svelte 5 event handler syntax for consistency.The component uses Svelte 5 runes (
$state,$effect,$props,$bindable), but the window event listener uses the legacy Svelte 4on:resizesyntax. For consistency, consider using the Svelte 5 syntax.♻️ Proposed fix
-<svelte:window on:resize={handleResize} /> +<svelte:window onresize={handleResize} />
167-167: Simplify the conditional class check.The
typeof noSqlEditor !== 'undefined'check is verbose. Snippets are truthy when defined, so a simple truthiness check suffices.♻️ Proposed fix
- class:has-json-editor={typeof noSqlEditor !== 'undefined'}> + class:has-json-editor={noSqlEditor}>
170-174: Consider simplifying the nested class structure.The outer
divat Line 170 has classno-sql-editor, and the innerdivat Line 172 has classno-sql-editor desktop. This redundancy could be confusing. Consider using justdesktopfor the inner div since it's already inside.no-sql-editor.♻️ Proposed fix
<div class="no-sql-editor"> {`#if` !$isSmallViewport} - <div class="no-sql-editor desktop" style:height={spreadsheetHeight}> + <div class="desktop" style:height={spreadsheetHeight}> {`@render` noSqlEditor?.()} </div>Then update the CSS selector accordingly:
- &:has(.no-sql-editor:empty), - &:has(.no-sql-editor.desktop:empty) { + &:has(.no-sql-editor:empty), + &:has(.no-sql-editor > .desktop:empty) { grid-template-columns: 1fr; }
...t-[region]-[project]/databases/database-[database]/(entity)/views/layouts/spreadsheet.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts:
- Around line 37-41: The getKeyName function's regex doesn't accept escaped
quotes inside quoted keys (e.g., "key\"name"), so update the pattern to allow
escaped characters and return the unescaped key; replace the current match regex
in getKeyName with one that captures escaped sequences such as
/^(?:"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)'|([A-Za-z_$][\w$]*))$/ and, if a
quoted capture is returned (match[1] or match[2]), unescape backslashes and
quotes (e.g., replace /\\(.)/g with '$1') before returning the key, otherwise
return the identifier capture (match[3]) or null.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte:
- Around line 36-43: The build fails because the file uses CodeMirror's Text
type (used at the occurrences around view editor logic) but doesn't import it,
causing TypeScript to pick the DOM Text type; fix this by adding an import of
Text from '@codemirror/state' (e.g., alongside EditorState, EditorSelection,
Transaction, Compartment, Extension) — use an `import type { Text } from
'@codemirror/state'` or include Text in the existing import so all usages (the
variables/functions referencing Text where methods like lineAt(), line(), or
iterChanges are called) use the correct CodeMirror Text type.
🧹 Nitpick comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts (3)
4-8: Export theSmartErrortype for consumers.
makeErrorMessagereturnsSmartError, but the type isn't exported. Callers cannot properly type variables holding the result.Suggested fix
-type SmartError = { +export type SmartError = { message: string; hint?: string; range?: { from: number; to: number }; };
193-193: Consider avoiding full document string copy for large documents.
state.doc.toString()creates a complete copy of the document in memory. For very large JSON documents, this adds memory pressure. CodeMirror'sTexttype supports efficient iteration viaiterLines()or character-by-character access without materializing the full string.
613-630: Consider unifying detector function signatures.The detectors array mixes functions that use
statedirectly with wrappers like(_state, ctx) => detectCommentToken(ctx). Sincestateis already used to buildcontext, consider having all detectors take onlyErrorContextand addingstateto the context if any detector truly needs it.This would simplify the array to a cleaner form:
const detectors: Array<(ctx: ErrorContext) => SmartError | null> = [ detectCommentToken, detectUnclosedStringAcrossLines, // ... ];src/routes/(console)/project-[region]-[project]/databases/database-[database]/store.ts (2)
68-68: Add explicit type annotation toexpandTabsstore.The store is initialized with
nullbut lacks a type annotation, which results in an inferred type ofWritable<null>. If this store is meant to hold other values, TypeScript won't catch type mismatches.✨ Suggested improvement
-export const expandTabs = writable(null); +export const expandTabs = writable<string | null>(null);Adjust the type (e.g.,
string,boolean, or a specific union) based on the expected values.
155-167: Consider adding explicit return type and aligning withSortState.The function's inferred return type may not align with the
SortStatetype defined above:
- Line 166 returns
column: null, butSortState.columnisstring | undefined(optional), notstring | null- The
direction: 'default'value assumesSortDirectionincludes'default'as a valid literalAdding an explicit return type would catch any mismatches at compile time.
✨ Suggested improvement
-export function extractSortFromQueries(parsedQueries: Map<TagValue, string>) { +export function extractSortFromQueries(parsedQueries: Map<TagValue, string>): SortState { for (const [tagValue, queryString] of parsedQueries.entries()) { if (queryString.includes('orderAsc') || queryString.includes('orderDesc')) { const isAsc = queryString.includes('orderAsc'); return { column: tagValue.value, - direction: isAsc ? 'asc' : 'desc' + direction: (isAsc ? 'asc' : 'desc') as SortDirection }; } } - return { column: null, direction: 'default' }; + return { column: undefined, direction: 'default' as SortDirection }; }Adjust based on whether
SortDirectionactually includes'default'and the expected semantics for "no column".src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte (1)
13-17: Prefer$routesalias for local route imports.
Switching the relative imports to$routeskeeps routing imports consistent and avoids path churn.♻️ Suggested update
-import { showCreateEntity, randomDataModalState, resetSampleFieldsConfig } from './store'; -import { entityColumnSuggestions } from './(suggestions)/store'; -import { showCreateBackup, showCreatePolicy } from './backups/store'; +import { + showCreateEntity, + randomDataModalState, + resetSampleFieldsConfig +} from '$routes/(console)/project-[region]-[project]/databases/database-[database]/store'; +import { entityColumnSuggestions } from '$routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store'; +import { showCreateBackup, showCreatePolicy } from '$routes/(console)/project-[region]-[project]/databases/database-[database]/backups/store';As per coding guidelines, use $lib, $routes, and $themes path aliases for imports.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/sonners/suggestions.svelte (1)
84-86: Drop the inline CSS comment to keep comments minimal.♻️ Suggested change
- & > :global:first-child { - display: unset; /* stack with flex-start in FAB */ + & > :global:first-child { + display: unset;As per coding guidelines: Use minimal comments; only comment TODOs or complex logic.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte (1)
52-77: Prefer $routes alias for local editor imports.These relative imports should use the $routes alias to comply with project import conventions. As per coding guidelines:
**/*.{js,ts,svelte}: Use $lib, $routes, and $themes path aliases for imports.
...ses/database-[database]/collection-[collection]/(components)/editor/helpers/errorMessages.ts
Show resolved
Hide resolved
...oject]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte:
- Around line 74-84: Replace hardcoded "table"/"table-" strings and static
labels with the existing terminology object: use terminology-provided labels
(e.g., terminology.labels.createEntity / terminology.labels.goToEntities /
terminology.labels.findEntities) instead of "Create table", "Go to tables",
"Find tables", and replace URL checks like page.url.pathname.includes('table-')
with a dynamic check using the terminology value (e.g.,
page.url.pathname.includes(`${terminology.entity}-`) or whichever terminology
field gives the entity slug) so the disabled logic and key hints use terminology
consistently; update all occurrences referenced in this diff (the callback that
sets $showCreateEntity and goto, the keys assignment, and the disabled
conditions at the shown lines and the other instances you noted) to reference
terminology and databaseId/project/base/page symbols already in scope.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte:
- Around line 930-943: The saved selection positions
(currentSelection.anchor/head) from the old document may be out-of-range after
replacing content, causing CodeMirror errors; in the block that builds the new
content (serializeData(updatedData)) and calls editorView.dispatch, remove the
explicit selection field so CodeMirror can automatically map the cursor through
the change instead of applying stale positions — update the dispatch call in the
editorView handling (where currentSelection/currentContent are captured and
editorView.dispatch is invoked) to only provide changes: { from: 0, to:
currentContent.length, insert: newContent } and omit selection.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/spreadsheet.svelte:
- Around line 280-329: hadErrors is unused dead code in handleDelete — remove it
and its conditional guard instead of keeping a flag that's never set; delete the
hadErrors declaration and the surrounding if (!hadErrors) { ... } block (which
wraps addNotification success), leaving the success notification in the try path
(so success is shown only when no exception occurs), and keep the existing catch
which already shows errors and short-circuits; update references to hadErrors
(if any) accordingly.
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/extensions/highlighting.ts (1)
117-167:createSystemFieldStylePluginscans all lines on every doc change — consider viewport scoping for large documents.Unlike
createNestedKeyPluginwhich only processes visible ranges, this plugin iterates every line in the document on each change. For typical document sizes this is fine, but if the editor could host very large JSON documents, consider scoping toview.visibleRangesthe same way.src/routes/(console)/project-[region]-[project]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte (1)
600-665:SvelteMapused in non-reactive contexts — use plainMapinstead.
checkSystemFieldsMatch(line 602) andapplySystemFieldsPatch(line 690) useSvelteMap, but these are local helper functions where Svelte reactivity tracking adds overhead for no benefit. A plainMapis more appropriate.♻️ Suggested fix
- const foundValues = new SvelteMap<string, string>(); + const foundValues = new Map<string, string>();- const hits = new SvelteMap<string, Hit>(); + const hits = new Map<string, Hit>();
src/routes/(console)/project-[region]-[project]/databases/database-[database]/+layout.svelte
Outdated
Show resolved
Hide resolved
...oject]/databases/database-[database]/collection-[collection]/(components)/editor/view.svelte
Show resolved
Hide resolved
...-[region]-[project]/databases/database-[database]/collection-[collection]/spreadsheet.svelte
Show resolved
Hide resolved
1 participant
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements