docs: add comprehensive GitHub Copilot onboarding instructions#1135
Conversation
Expands .github/copilot-instructions.md from a minimal package manager note into a full agent onboarding guide covering project purpose, architecture, API reference, DataPath syntax, error codes, encryption, testing conventions, commit format, CI/CD pipeline, and known gotchas. Co-authored-by: Belphemur <197810+Belphemur@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1135 +/- ##
========================================
Coverage 99.60% 99.60%
========================================
Files 12 12
Lines 1531 1531
Branches 226 322 +96
========================================
Hits 1525 1525
Misses 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Expands .github/copilot-instructions.md from a minimal pnpm note into a comprehensive onboarding/reference guide for Copilot coding agents working on node-json-db.
Changes:
- Adds a project overview and annotated architecture/directory tree.
- Documents public API, DataPath syntax, error codes, encryption details, and conventions (TS/testing/commits/CI).
- Includes adapter implementation guidance and a “known gotchas” section.
| import { IAdapter } from 'node-json-db' | ||
|
|
||
| class MyAdapter implements IAdapter<string> { | ||
| async readAsync(): Promise<string | null> { | ||
| // Return raw string data or null if not found | ||
| } | ||
| async writeAsync(data: string): Promise<void> { | ||
| // Persist raw string data | ||
| } | ||
| } | ||
|
|
||
| // Use with ConfigWithAdapter: | ||
| import { JsonDB, ConfigWithAdapter } from 'node-json-db' | ||
| const db = new JsonDB(new ConfigWithAdapter(new JsonAdapter(new MyAdapter()))) | ||
| ``` |
There was a problem hiding this comment.
The Adapter Pattern example uses new JsonAdapter(...) but the snippet doesn’t import JsonAdapter, so it won’t compile as written. Either add JsonAdapter to the import list or show an example where the custom adapter already reads/writes JSON and can be passed directly to ConfigWithAdapter.
| 1. **`push()` with `override=false`** merges recursively for objects, concatenates for arrays, and skips `null` values | ||
| 2. **`getObjectDefault<T>()`** catches only `DataError` with `id === 5` (path not found); other errors still propagate |
There was a problem hiding this comment.
Gotcha #1 says push() merge “skips null values”, but the merge() implementation in src/lib/Utils.ts does not skip nulls—source[prop] can overwrite with null, and only the special-case is when the destination is null. Please adjust this description to match the actual merge semantics (arrays concat; objects recurse; primitives including null overwrite).
| 5. **Concurrency**: `readLockAsync`/`writeLockAsync` from `src/lock/Lock.ts` wrap all public read/write operations; custom adapter implementations do not need to add their own locking | ||
| 6. **Date parsing**: Enabled by default (`parseDates: true`); ISO 8601 strings are automatically revived as `Date` objects |
There was a problem hiding this comment.
Gotcha #5 claims readLockAsync/writeLockAsync wrap all public read/write operations, but load(), save(), reload(), fromPath(), and resetData() are not lock-wrapped in src/JsonDB.ts. Consider narrowing this statement to the specific methods that are locked (getData/push/delete, and methods that call getData).
| All methods are async and protected by reader-writer locks internally. | ||
|
|
There was a problem hiding this comment.
The statement “All methods are async and protected by reader-writer locks internally” isn’t accurate for the current JsonDB API: resetData() is synchronous, and load(), save(), reload(), and fromPath() are not wrapped in readLockAsync/writeLockAsync. Please adjust this wording to reflect which methods are actually lock-protected/async to avoid misleading API consumers and agents.
| // id 12: filter/find on non-array/non-object | ||
| // id 13: fromPath value not found | ||
| // id 100: Can't get/delete appended data | ||
| // id 200: Non-numeric array index | ||
|
|
There was a problem hiding this comment.
DataError id mappings here don’t match the implementation in a few places: id 100 is specifically “Can’t get data when appending” (not delete), and id 10 is also used for “Can’t delete an appended data” in ArrayInfo.delete(). Please align these descriptions with the actual throw sites to keep the error-code reference reliable.
| 7. **File auto-created**: The JSON file is only created on the first `save()` (triggered by `push()`/`delete()` with `saveOnPush: true`); the file does NOT exist after a bare `new JsonDB(config)` call | ||
| 8. **`fromPath()`**: Converts Express-style routes (e.g., `/users/123`) to DataPaths using `getIndex()` internally |
There was a problem hiding this comment.
Gotcha #7 says the JSON file is created only on the first save() (via push/delete), but the default file-based adapter path will also create the file during the first load()/getData() when it doesn’t exist (because JsonAdapter.readAsync() writes {} on empty/missing data). Please update this to reflect that the first read can create the file.
| - **DataPath navigation**: XPath-like syntax (`/users/0/name`, `/items[]`, `/data[-1]`) to navigate nested JSON data | ||
| - **Async-first API**: All database operations return Promises (async/await) |
There was a problem hiding this comment.
The DataPath examples use /users/0/name, but the project’s documented/featured array syntax uses bracket notation (e.g. /users[0]/name) to support [] append and [-1] semantics consistently (see README array examples). Consider updating this example to bracket form to avoid implying /0 is the recommended array access pattern.
| | `/users` | `users` property of root | | ||
| | `/users/0/name` | `name` of first user | | ||
| | `/items[0]` | First element of `items` array | |
There was a problem hiding this comment.
This table entry uses /users/0/name, which is inconsistent with the library’s standard array indexing syntax (/users[0]/name) used throughout the README and tests. Using bracket notation here will better match how ArrayInfo parses arrays (and avoids confusion with object property names that happen to be numeric strings).
| // DataError — problems with path or data (ids 3–13, 100, 200) | ||
| // id 3: Can't merge another type with Array | ||
| // id 4: Can't merge Array with Object | ||
| // id 5: Path not found (used by getObjectDefault to return default) | ||
| // id 10: Can't find array index |
There was a problem hiding this comment.
This section says it documents all DataError ids 3–13, 100, 200, but it currently omits id 6 (The Data Path can't be empty). Also, ids 7–9 aren’t used in the codebase, so the “3–13” range wording is misleading—consider listing only the ids that actually exist.
| ### `codeql-analysis.yml` — Security Scanning | ||
| - Runs CodeQL analysis on pushes/PRs to master/develop | ||
|
|
There was a problem hiding this comment.
The CodeQL workflow description says it runs on PRs to both master and develop, but .github/workflows/codeql-analysis.yml is configured to run on pull requests targeting develop only. Please update this line to match the actual triggers so agents don’t assume CodeQL runs on every PR to master.
|
🎉 This PR is included in version 2.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The existing
.github/copilot-instructions.mdonly covered pnpm setup (44 lines). This expands it into a full agent onboarding reference so Copilot coding agents can work effectively without repeated exploration.Changes to
.github/copilot-instructions.mdsrc/file; adapter chain (JsonDB → JsonAdapter → CipheredFileAdapter | FileAdapter) made explicitJsonDB;Config/ConfigWithAdapterconstructor signatures with all parameters and defaults/,/prop,[0],[],[-1],[0][1],[]/propDataError(ids 3–13, 100, 200) andDatabaseError(ids 1, 2, 7) numeric codes documented.enc.jsonextension behavior, on-disk format (iv/tag/data)_prefixfor private,PascalCase/camelCase), JSDoc requirement for public APIafterEachwithrmSync/unlinkSync),/tmp/<uuid>pattern for encrypted DB tests, numbered = integration / unnumbered = unitcommitlint;body-max-line-lengthdisablednodejs.yml,release.yml,codeql-analysis.yml);semantic-releaseflow summarizedIAdapter<string>implementation guide withConfigWithAdapterusagegetObjectDefaulterror id filtering, file creation timing,ArrayInforegex caching, lock scope)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.