add data importer #1719
add data importer #1719
Conversation
Import legacy JSON into store on startup Add a minimal import routine that reads hyprnote/import/main.json, parses its tables and values, merges them into the current MergeableStore, and then removes the file. This is needed to support migrating data when load/merge is performed and to allow removing the old JSON import path once merged. The code also handles errors gracefully and is invoked after persister.load() during store initialization.
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a new TypeScript import utility that reads a JSON export from the Tauri data directory and merges it into a TinyBase store; the main TinyBase module exposes its SCHEMA and invokes the importer during persister initialization. Also excludes Netlify edge functions from the web tsconfig include. Changes
Sequence Diagram(s)sequenceDiagram
participant Persister as Persister Init
participant Importer as maybeImportFromJson
participant FS as Tauri FS
participant ImportStore as Import TinyBase Store
participant MainStore as Main TinyBase Store
Persister->>Importer: call maybeImportFromJson(store)
Importer->>FS: stat/read "hyprnote/import/main.json"
alt file exists
FS-->>Importer: file contents (JSON)
Importer->>Importer: parse → [tables, values]
Importer->>ImportStore: create mergeable store with SCHEMA
Importer->>ImportStore: apply tables & values (if present)
Importer->>MainStore: merge(importStore -> main store)
Importer->>FS: delete "hyprnote/import/main.json"
Importer-->>Persister: resolved
else not found / error
FS-->>Importer: not found or read error
Importer-->>Persister: resolved (no-op) / logs error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (1)
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 |
![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)
apps/desktop/src/store/tinybase/main.ts (1)
28-28: Consider breaking the circular dependency.The import creates a circular dependency:
main.tsimportsimporter.ts, which importsSCHEMAfrommain.ts. While this may work, consider extractingSCHEMAto a separate file (e.g.,schema.ts) to eliminate the cycle and improve modularity.apps/desktop/src/store/tinybase/importer.ts (1)
12-12: Consider extracting the magic string.The path
"hyprnote/import/main.json"is hardcoded. Consider extracting it as a named constant for better maintainability and to make it easier to locate if the path needs to change.+const IMPORT_FILE_PATH = "hyprnote/import/main.json"; + export const maybeImportFromJson = async (store: Store) => { - const path = "hyprnote/import/main.json"; + const path = IMPORT_FILE_PATH; const baseDir = BaseDirectory.Data;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/store/tinybase/importer.ts(1 hunks)apps/desktop/src/store/tinybase/main.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/desktop/src/store/tinybase/importer.ts (1)
apps/desktop/src/store/tinybase/main.ts (2)
Store(74-74)SCHEMA(39-47)
apps/desktop/src/store/tinybase/main.ts (1)
apps/desktop/src/store/tinybase/importer.ts (1)
maybeImportFromJson(11-38)
⏰ 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). (1)
- GitHub Check: fmt
🔇 Additional comments (3)
apps/desktop/src/store/tinybase/main.ts (2)
39-47: LGTM!Exposing
SCHEMAas a public export is necessary for the importer module to use it, and the schema definition remains correct.
123-123: Verify transaction boundaries and error handling.The import occurs before the transaction starts on line 124, meaning the merge operation is not atomic with the initialization logic. Additionally, if the import fails, the error is only logged in the importer module and execution continues.
Consider:
- Moving the import call inside the transaction callback to ensure atomicity
- Propagating errors from
maybeImportFromJsonif import failures should halt initializationapps/desktop/src/store/tinybase/importer.ts (1)
1-9: LGTM!The imports are appropriate for the functionality provided. The circular dependency with
main.tsis addressed in the main.ts review.
| const content = await readTextFile(path, { baseDir }); | ||
| const [tables, values] = JSON.parse(content) as [unknown, unknown]; |
There was a problem hiding this comment.
Add runtime validation for JSON structure.
The code assumes the JSON content is a two-element array but doesn't validate this structure. If the file contains malformed data (e.g., a single object, an array with a different length, or invalid JSON), the destructuring may fail silently or produce unexpected results.
const content = await readTextFile(path, { baseDir });
-const [tables, values] = JSON.parse(content) as [unknown, unknown];
+const parsed = JSON.parse(content);
+if (!Array.isArray(parsed) || parsed.length !== 2) {
+ throw new Error("Invalid import file format: expected [tables, values] tuple");
+}
+const [tables, values] = parsed as [unknown, unknown];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const content = await readTextFile(path, { baseDir }); | |
| const [tables, values] = JSON.parse(content) as [unknown, unknown]; | |
| const content = await readTextFile(path, { baseDir }); | |
| const parsed = JSON.parse(content); | |
| if (!Array.isArray(parsed) || parsed.length !== 2) { | |
| throw new Error("Invalid import file format: expected [tables, values] tuple"); | |
| } | |
| const [tables, values] = parsed as [unknown, unknown]; |
🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/importer.ts around lines 20-21, the code
blindly parses JSON and destructures it into [tables, values]; add runtime
validation and error handling: wrap JSON.parse in try/catch to surface invalid
JSON, verify the parsed value is an array with exactly two elements, and confirm
each element is the expected type (e.g., objects or arrays as your consumer
expects); if validation fails, throw or return a clear error so callers can
handle malformed files instead of proceeding with undefined or incorrect data.
| if (tables) { | ||
| importStore.setTables(tables as never); | ||
| } | ||
| if (values) { | ||
| importStore.setValues(values as never); | ||
| } |
There was a problem hiding this comment.
Improve type safety with schema validation.
Using as never type assertions bypasses all TypeScript type checking. If the imported data doesn't match SCHEMA, it could lead to runtime errors or data corruption. While TinyBase may perform runtime schema validation, the type assertions eliminate compile-time safety.
Consider validating the imported data against the schema or using TinyBase's built-in validation mechanisms more explicitly.
🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/importer.ts around lines 27-32, the use of
"as never" on imported tables/values bypasses TypeScript checking and can allow
schema-mismatched data into the store; replace the assertions with proper
validation: validate the imported object shape against your SCHEMA at runtime
(using TinyBase's import/validation helpers or a lightweight validator like
zod/io-ts) and only call importStore.setTables/setValues with typed data after
successful validation, or cast to the correct typed interfaces rather than
"never", and log/throw a clear error on validation failure so invalid imports
are rejected before mutating the store.
| store.merge(importStore); | ||
| await remove(path, { baseDir }); | ||
| } catch (error) { | ||
| console.error(error); | ||
| } |
There was a problem hiding this comment.
Clarify error handling and cleanup strategy.
Current behavior:
- If the merge fails, the file isn't removed, causing retry on next startup
- If the merge succeeds but file removal fails, the error is logged but the file remains, also causing retry
- All errors are caught and logged, but not propagated, making failures invisible to the caller
Consider:
- Remove the file before merging (if you want to prevent retries of malformed data)
- Or ensure the file is removed even if merge fails (using try-finally)
- Or propagate errors to the caller for explicit handling
- Add success logging for observability
Example with try-finally for guaranteed cleanup:
try {
const content = await readTextFile(path, { baseDir });
const [tables, values] = JSON.parse(content) as [unknown, unknown];
const importStore = createMergeableStore()
.setTablesSchema(SCHEMA.table)
.setValuesSchema(SCHEMA.value);
if (tables) {
importStore.setTables(tables as never);
}
if (values) {
importStore.setValues(values as never);
}
+
+ // Remove file before merging to prevent retry of malformed data
+ await remove(path, { baseDir });
+
store.merge(importStore);
- await remove(path, { baseDir });
+ console.log("Successfully imported data from", path);
} catch (error) {
console.error("Failed to import data:", error);
+ // Consider whether to rethrow or handle differently
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/importer.ts around lines 33-37, the current
catch-only error handling causes files to remain on failure and swallows errors;
change to guarantee cleanup and surface failures by using a try { await
store.merge(importStore); } finally { await remove(path, { baseDir }).catch(err
=> processLogger.error('Failed to remove import file', { path, err })); } and
rethrow any merge error so the caller can handle it (or alternatively remove the
file before merging if you prefer to avoid retries of malformed data), and add a
success log after merge completes.
Avoid TypeScript errors from Netlify edge function files by excluding the netlify/edge-functions directory from the apps/web tsconfig include. The edge functions reference remote ESM modules and Deno-specific types that cause "Cannot find module" and implicit any errors during the web project's TypeScript build, so excluding them prevents these spurious compile failures. - Add "netlify/edge-functions/**/*" to tsconfig.json exclude to skip compiling those files.
No description provided.