better ts lint #465
better ts lint #465
Conversation
mabels
force-pushed
the
mabels/better-ts-lint
branch
from
9530fc0 to
09f20c5
Compare
WalkthroughThis pull request introduces a comprehensive set of type refinements, code simplifications, and configuration updates across the Fireproof project. The changes primarily focus on enhancing type safety, removing unnecessary type assertions, and streamlining type definitions. Key modifications include updating ESLint configurations, adjusting TypeScript configurations, and making subtle changes to type handling in various source and test files. The modifications aim to improve code clarity, reduce potential runtime errors, and leverage TypeScript's type inference capabilities more effectively. Changes
Sequence DiagramsequenceDiagram
participant Developer
participant TypeScript
participant ESLint
participant Project
Developer->>TypeScript: Modify type definitions
TypeScript->>Project: Apply stricter type checking
Project->>ESLint: Enforce new type rules
ESLint->>Developer: Provide type-related feedback
Developer->>Project: Refine code based on feedback
This diagram illustrates the iterative process of type refinement, where developers modify type definitions, TypeScript applies stricter checking, ESLint enforces rules, and developers further refine the code based on the feedback. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
src/version.ts (1)
Simplify version constant and use proper version management.
The current implementation using
Object.keys()is unnecessarily complex for a version constant that's only used for logging purposes. The "xxxx" value is indeed a placeholder.
- Consider using the version from package.json during build time
- If build-time replacement isn't needed, use a direct string as suggested in the review
🔗 Analysis chain
Line range hint
1-4: Simplify package version implementation and improve type safety.The current implementation is unnecessarily complex and could be simplified while maintaining type safety.
Consider this alternative implementation:
-export const PACKAGE_VERSION = Object.keys({ - __packageVersion__: "xxxx", -})[0]; +export const PACKAGE_VERSION = "__packageVersion__";Also, the value "xxxx" suggests this might be a placeholder. Consider using a more meaningful version string or implementing dynamic version retrieval from package.json.
Let's verify if this is indeed a placeholder:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there's a proper version management system in place rg -l "packageVersion|PACKAGE_VERSION" package.jsonLength of output: 53
Script:
#!/bin/bash # Check if package.json exists and show its content echo "=== Checking package.json existence and content ===" fd -t f "^package.json$" -x cat {} # Search for version-related patterns in the codebase echo -e "\n=== Searching for version patterns ===" rg -l "version|VERSION" --type ts # Find usages of PACKAGE_VERSION echo -e "\n=== Finding PACKAGE_VERSION usages ===" rg "PACKAGE_VERSION" --type ts -A 2Length of output: 14022
🧹 Nitpick comments (10)
src/blockstore/commit-queue.ts (1)
51-51: Fix code style issue.The added block syntax in the map callback triggered a Prettier warning.
Apply this diff to fix the style issue:
- toResolve.map((fn) => { fn.resolve(); }); + toResolve.map((fn) => fn.resolve());🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/runtime/gateways/fp-envelope-serialize.ts (1)
37-37: LGTM! Type simplification aligns with PR objectives.The simplification of the type cast from
Link<EventView<DbMetaBinary>, number, number, 1>[]toLink<EventView<DbMetaBinary>>[]is a good improvement. It reduces type complexity while maintaining type safety.Consider removing the
as unknownpart of the type cast if possible, as it bypasses TypeScript's type checking. If the types are truly compatible, the cast should work withoutunknown.src/crdt-helpers.ts (1)
151-153: Consider using a more specific type assertion.The
as unknowntype assertion is too broad and might mask type incompatibilities.Consider creating a specific type for the files parameter:
- { files } as unknown /* { - public: publicFiles, - } */, + { files } as { files: Record<string, DocFileMeta> },🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/ledger.ts (1)
389-389: Consider simplifying the async error handling.The async IIFE with block scope adds unnecessary complexity.
Consider using a simpler try-catch pattern:
- await (async () => { await listener(docs as DocWithId<DT>[]); })().catch((e: Error) => { + try { + await listener(docs as DocWithId<DT>[]); + } catch (e: Error) { this.logger.Error().Err(e).Msg("subscriber error"); - }); + }Also applies to: 400-400
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/blockstore/store.ts (1)
146-146: Consider keeping the simpler closure syntax.The change from
fn()to{ fn(); }adds unnecessary verbosity without providing additional benefits. Consider keeping the simpler syntax for better readability.- this._onStarted.forEach((fn) => { fn(); }); + this._onStarted.forEach((fn) => fn());Also applies to: 260-260, 336-336, 577-577
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/write-queue.ts (1)
42-44: Run Prettier to maintain consistent code style.The addition of curly braces to the forEach callbacks is a style change. As indicated by the pipeline failures, run Prettier with --write to ensure consistent formatting across the codebase.
-tasksToProcess.forEach((task) => { task.resolve(result); }); +tasksToProcess.forEach((task) => task.resolve(result)); -tasksToProcess.forEach((task) => { task.reject(error as Error); }); +tasksToProcess.forEach((task) => task.reject(error as Error));🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
tsconfig.json (2)
116-128: Optimize exclude patterns to reduce redundancy.The exclude patterns can be simplified:
"**/scripts/"already covers"scripts/", making the latter redundant- Consider grouping related patterns (e.g., test configs, build outputs) using wildcards
"exclude": [ - "babel.config.cjs", - "jest.config.js", + "*.config.*", "dist/", "**/pubdir/", "node_modules/", "**/scripts/", "**/examples/", - "scripts/", "coverage/", "smoke/react/", "src/missingTypes/lib.deno.d.ts" ]🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
116-128: Fix code formatting with Prettier.The pipeline indicates code style issues. Run Prettier to automatically fix the formatting:
prettier --write tsconfig.json🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
tests/fireproof/fireproof.test.ts (2)
683-688: Consider safer type handling for blockstore loader access.While removing optional chaining aligns with the PR's goal of improving type safety, we should ensure proper type narrowing to handle potential undefined values safely.
Consider this safer approach:
- const logA = dbA.crdt.blockstore.loader.carLog; - expect(logA).toBeTruthy(); - assert(logA); + const loader = dbA.crdt.blockstore.loader; + expect(loader).toBeDefined(); + const logA = loader.carLog; + expect(logA).toBeDefined();
705-710: Apply consistent type handling for compacted logs.Similar to the previous comment, the compacted logs section should follow the same pattern of safe type handling.
Consider this safer approach:
- const cmpLogA = dbA.crdt.blockstore.loader.carLog; - expect(cmpLogA).toBeTruthy(); - assert(cmpLogA); + const loader = dbA.crdt.blockstore.loader; + expect(loader).toBeDefined(); + const cmpLogA = loader.carLog; + expect(cmpLogA).toBeDefined();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
eslint.config.mjs(2 hunks)src/blockstore/commit-queue.ts(2 hunks)src/blockstore/loader.ts(2 hunks)src/blockstore/store.ts(10 hunks)src/blockstore/transaction.ts(1 hunks)src/blockstore/types.ts(2 hunks)src/crdt-clock.ts(1 hunks)src/crdt-helpers.ts(3 hunks)src/crdt.ts(1 hunks)src/ledger.ts(5 hunks)src/react/useFireproof.ts(3 hunks)src/runtime/files.ts(1 hunks)src/runtime/gateways/file/node-filesystem.ts(1 hunks)src/runtime/gateways/fp-envelope-serialize.ts(1 hunks)src/runtime/gateways/indexdb/gateway.ts(3 hunks)src/runtime/key-bag-indexdb.ts(2 hunks)src/runtime/keyed-crypto.ts(2 hunks)src/runtime/wait-pr-multiformats/block.ts(2 hunks)src/version.ts(1 hunks)src/write-queue.ts(1 hunks)tests/fireproof/all-gateway.test.ts(4 hunks)tests/fireproof/fireproof.test.ts(2 hunks)tests/fireproof/ledger.test.ts(7 hunks)tests/fireproof/utils.test.ts(1 hunks)tests/gateway/file/loader-config.test.ts(13 hunks)tests/gateway/fp-envelope-serialize.test.ts(1 hunks)tests/gateway/indexdb/loader-config.test.ts(4 hunks)tests/helpers.ts(1 hunks)tests/react/useFireproof.test.tsx(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/crdt-clock.ts
🧰 Additional context used
🪛 GitHub Actions: CI
src/write-queue.ts
[warning] Code style issues found. Run Prettier with --write to fix.
tests/fireproof/all-gateway.test.ts
[warning] Code style issues found. Run Prettier with --write to fix.
src/blockstore/commit-queue.ts
[warning] Code style issues found. Run Prettier with --write to fix.
tsconfig.json
[warning] Code style issues found. Run Prettier with --write to fix.
src/crdt-helpers.ts
[warning] Code style issues found. Run Prettier with --write to fix.
eslint.config.mjs
[warning] Code style issues found. Run Prettier with --write to fix.
src/blockstore/store.ts
[warning] Code style issues found. Run Prettier with --write to fix.
src/ledger.ts
[warning] Code style issues found. Run Prettier with --write to fix.
🪛 eslint
src/runtime/gateways/file/node-filesystem.ts
[error] 26-26: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 26-26: Optional chain expressions can return undefined by design - using a non-null assertion is unsafe and wrong.
(@typescript-eslint/no-non-null-asserted-optional-chain)
tests/fireproof/ledger.test.ts
[error] 426-426: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 437-437: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 448-448: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 467-467: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 484-484: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
src/runtime/gateways/indexdb/gateway.ts
[error] 43-43: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
src/react/useFireproof.ts
[error] 259-259: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
🔇 Additional comments (29)
tests/react/useFireproof.test.tsx (1)
16-16: Verify ledger initialization guarantees.Removing optional chaining (
?.) makes the test more strict but potentially brittle. Before this change, the test would handle undefined ledger gracefully.Let's verify the ledger initialization guarantees:
✅ Verification successful
Optional chaining removal is safe - ledger initialization is guaranteed
The ledger is always initialized in useFireproof through direct assignment, with guaranteed values from either the fireproof() call or the passed Ledger instance. The removal of optional chaining aligns with the actual implementation guarantees.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check ledger initialization patterns ast-grep --pattern 'const { ledger } = useFireproof($_)' # Check if ledger can be undefined in the implementation rg -A 5 'export function useFireproof'Length of output: 1248
src/blockstore/commit-queue.ts (1)
6-6: LGTM! Type broadening improves flexibility.Removing the explicit
voidtype constraint allows the queue to handle functions returning promises of any type, which better aligns with the generic type parameterTin the class definition.🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
eslint.config.mjs (4)
7-11: LGTM! Enhanced type checking configuration.Switching to type-checked variants of the ESLint configurations (
strictTypeChecked,stylisticTypeChecked,recommendedTypeChecked) improves type safety enforcement.🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
41-44: LGTM! Separate configuration for JavaScript files.Good practice to disable type checking for JavaScript files while maintaining it for TypeScript files.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
53-53: LGTM! Enforcing immutability.Adding
@typescript-eslint/prefer-readonlyrule as an error aligns well with the goal of improving type safety and preventing accidental mutations.🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
13-19: Verify TypeScript project configuration.The configuration now explicitly references the TypeScript project settings.
Let's verify the TypeScript configuration:
✅ Verification successful
TypeScript project configuration is correctly set up ✅
The TypeScript configuration is properly integrated with ESLint and includes all necessary compiler options, with strict type checking enabled and appropriate module resolution settings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if tsconfig.json exists and contains necessary compiler options cat tsconfig.json | jq '.compilerOptions' # Verify the project root directory resolution pwdLength of output: 123
Script:
#!/bin/bash # Check if tsconfig exists and show its content echo "=== TypeScript Config Files ===" fd -e json -e jsonc tsconfig echo echo "=== tsconfig.json content ===" [ -f tsconfig.json ] && cat tsconfig.json echo echo "=== package.json TypeScript references ===" [ -f package.json ] && cat package.json | grep -i typescriptLength of output: 13473
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/runtime/key-bag-indexdb.ts (1)
8-8: LGTM! Type simplification looks good.The removal of the redundant
<unknown>type parameter fromIDBPDatabaseimproves code clarity while maintaining type safety.Also applies to: 21-21
tests/gateway/indexdb/loader-config.test.ts (1)
19-20: LGTM! Removal of unnecessary optional chaining.The removal of optional chaining operators improves code clarity, as these methods are guaranteed to exist on the loader object.
Also applies to: 33-34, 47-48, 60-61
src/runtime/files.ts (1)
60-60: LGTM! Enhanced immutability with readonly modifier.Adding the
readonlymodifier to the private field#fileimproves type safety by preventing accidental reassignment after initialization.src/crdt.ts (1)
197-197: LGTM!The change correctly implements the
Promise<void>return type by awaiting the operation without returning a value.src/runtime/gateways/indexdb/gateway.ts (1)
16-16: LGTM!The removal of the generic type parameter from
IDBPDatabasesimplifies the type definition without impacting functionality.Also applies to: 94-94
src/runtime/keyed-crypto.ts (2)
45-45: LGTM!The change to dot notation is more idiomatic when accessing known property names.
88-88: LGTM!Removing optional chaining is safe here as
optsis initialized in the constructor with an empty object as fallback.tests/gateway/file/loader-config.test.ts (1)
40-41: LGTM! Removal of optional chaining improves type safety.The removal of optional chaining (
?.) from method calls indicates that these methods are expected to be always available. This change improves type safety by making the assumptions about method existence explicit.Let's verify that these methods are indeed required properties of the loader:
Also applies to: 55-56, 70-71, 84-85, 117-118, 131-132, 145-146, 183-185, 202-203, 216-217, 253-254, 270-271, 285-286
✅ Verification successful
Verified: The loader methods are required properties
The type definitions in
src/blockstore/types.tsand the implementation insrc/blockstore/loader.tsconfirm thatfileStore()and other loader methods are required (non-optional) properties that always return a Promise of their respective store types. The removal of optional chaining correctly reflects the type system's guarantees.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the loader methods are required properties in the type definition. # Search for the loader interface/type definition ast-grep --pattern 'interface $_ { $$$ fileStore(): $_ carStore(): $_ metaStore(): $_ WALStore(): $_ $$$ }'Length of output: 121
Script:
#!/bin/bash # Search for type definitions containing fileStore ast-grep --pattern 'interface $name { $$$ fileStore$_ $$$ }' # Also try with type aliases ast-grep --pattern 'type $name = { $$$ fileStore$_ $$$ }' # Search for the actual implementation rg "fileStore.*\(" -A 2Length of output: 7288
tests/gateway/fp-envelope-serialize.test.ts (1)
240-243: LGTM! Improved error handling and type safety.The changes enhance the code in multiple ways:
- Added explicit type definition for
jsonimproves type safety- Added error handling for empty metadata prevents potential issues
- Simplified type assertion in the mapping function aligns with the PR objectives
Also applies to: 246-248
src/react/useFireproof.ts (1)
279-279: Verify type safety after removing type assertions.The removal of type assertions
as DocWithId<T>might expose type incompatibilities. Ensure thatr.valuematches the expectedDocWithId<T>type.Run this script to verify the type compatibility:
Also applies to: 299-299
✅ Verification successful
Type assertions can be safely removed
The type safety is maintained because the database query operations ensure that
r.valueinherently matches theDocWithId<T>structure. The query results are properly typed at their source, making the type assertions redundant.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if r.value matches DocWithId<T> type structure # Look for value property usage in test files and implementations ast-grep --pattern 'interface DocWithId<T> { $$$ }' rg -A 5 'r\.value'Length of output: 4627
Script:
#!/bin/bash # Find DocWithId definition and related types rg "DocWithId" -A 5 # Check imports in useFireproof.ts rg "import.*DocWithId" # Look for type definitions in d.ts files fd -e d.ts -x cat {}Length of output: 82413
src/crdt-helpers.ts (1)
131-131: LGTM! Type assertion removal improves type safety.The removal of the type assertion is correct since the code already checks the constructor type using
File === files[filename].constructor.🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/blockstore/types.ts (2)
27-27: LGTM! Type simplification improves readability.The removal of generic parameters from the CID type simplifies the type definition without losing type safety.
360-360: LGTM! Type parameter removal is appropriate.The removal of the void type parameter from CommitQueue is correct since it's not needed for the queue's functionality.
src/ledger.ts (3)
147-147: LGTM! Removed unnecessary return statement.The function is correctly declared as void, and removing the return statement improves code clarity.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
228-228: LGTM! Added block scope to callback execution.The addition of block scope to the callback execution improves code consistency.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
299-299: LGTM! Removed optional chaining.The removal of optional chaining is appropriate since
result.headis guaranteed to exist in this context.Also applies to: 306-306
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
tests/fireproof/all-gateway.test.ts (1)
53-56: LGTM! Type assertions removed correctly.The removal of type assertions aligns with the PR objectives. The types are correctly inferred from the loader methods, making the code more maintainable and type-safe.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
src/blockstore/loader.ts (1)
75-77: LGTM! Properties correctly marked as readonly.The addition of the
readonlymodifier to private properties prevents accidental modifications after initialization, improving type safety and encapsulation.src/blockstore/store.ts (2)
114-114: LGTM! Simplified error handling.The removal of unnecessary type assertions in error handling makes the code cleaner while maintaining type safety.
Also applies to: 129-129
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
157-157: LGTM! Improved type casting.The updated type casting for the parents parameter aligns with the PR objectives to improve type safety.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
tests/fireproof/ledger.test.ts (1)
174-175: LGTM! Simplified string literals.The change from template literals to simple string literals for fixed strings improves readability.
Also applies to: 188-189, 193-193
tests/helpers.ts (1)
57-57: LGTM! Improved default value handling.The change from
||to??operator is a good improvement as it only falls back to "test" whennameis null or undefined, preserving other falsy values like empty strings.src/runtime/wait-pr-multiformats/block.ts (1)
28-28: LGTM! Removed unnecessary type assertions.The removal of type assertions from
CID.createcalls is a good improvement as the types are correctly inferred from the generic parameters. This makes the code cleaner while maintaining type safety.Also applies to: 49-49
| @@ -23,7 +23,7 @@ export class NodeFileSystem implements SysFileSystem { | |||
| return this.fs?.mkdir(path, options); | |||
| } | |||
| async readdir(path: PathLike, options?: ObjectEncodingOptions): Promise<string[]> { | |||
| return this.fs?.readdir(path, options) as Promise<string[]>; | |||
| return this.fs?.readdir(path, options)!; | |||
There was a problem hiding this comment.
Remove unsafe non-null assertion with optional chaining.
Using non-null assertion (!) with optional chaining (?.) is unsafe as it defeats the purpose of optional chaining. The fs property could be undefined if start() hasn't been called.
Consider this safer alternative:
- return this.fs?.readdir(path, options)!;
+ if (!this.fs) {
+ throw new Error('FileSystem not initialized. Call start() first.');
+ }
+ return this.fs.readdir(path, options);📝 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.
| return this.fs?.readdir(path, options)!; | |
| if (!this.fs) { | |
| throw new Error('FileSystem not initialized. Call start() first.'); | |
| } | |
| return this.fs.readdir(path, options); |
🧰 Tools
🪛 eslint
[error] 26-26: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 26-26: Optional chain expressions can return undefined by design - using a non-null assertion is unsafe and wrong.
(@typescript-eslint/no-non-null-asserted-optional-chain)
| @@ -113,7 +113,7 @@ describe("utils", () => { | |||
|
|
|||
| describe("runtime", () => { | |||
| it("runtime", () => { | |||
| const isNode = !!(typeof process === "object" && process.versions?.node); | |||
| const isNode = !!(typeof process === "object" && process.versions.node); | |||
There was a problem hiding this comment.
Consider keeping the optional chaining for runtime safety.
Removing optional chaining from process.versions.node could cause runtime errors in environments where process exists but versions is undefined. The optional chaining provides a safer way to handle this.
- const isNode = !!(typeof process === "object" && process.versions.node);
+ const isNode = !!(typeof process === "object" && process.versions?.node);📝 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 isNode = !!(typeof process === "object" && process.versions.node); | |
| const isNode = !!(typeof process === "object" && process.versions?.node); |
| @@ -40,7 +40,7 @@ async function connectIdb(url: URI, sthis: SuperThis): Promise<IDBConn> { | |||
| }, | |||
| }); | |||
| const found = await db.get("version", "version"); | |||
| const version = ensureVersion(url).getParam(PARAM.VERSION) as string; | |||
| const version = ensureVersion(url).getParam(PARAM.VERSION)!; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace non-null assertion with explicit null check.
Instead of using the non-null assertion operator, consider adding an explicit null check for better type safety.
- const version = ensureVersion(url).getParam(PARAM.VERSION)!;
+ const version = ensureVersion(url).getParam(PARAM.VERSION);
+ if (!version) {
+ throw new Error('Version parameter is required');
+ }📝 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 version = ensureVersion(url).getParam(PARAM.VERSION)!; | |
| const version = ensureVersion(url).getParam(PARAM.VERSION); | |
| if (!version) { | |
| throw new Error('Version parameter is required'); | |
| } |
🧰 Tools
🪛 eslint
[error] 43-43: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
| @@ -66,7 +66,7 @@ export function defaultedBlockstoreRuntime( | |||
| }, | |||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |||
| compact: async (blocks: BlockFetcher) => { | |||
| return {} as unknown as TransactionMeta; | |||
| return {} as unknown; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more specific type instead of unknown.
The change to return {} as unknown weakens type safety. If this method is expected to return a specific type, consider maintaining that type or documenting why a generic unknown type is necessary.
If the original TransactionMeta type is still valid, consider keeping it:
- return {} as unknown;
+ return {} as TransactionMeta;Committable suggestion skipped: line range outside the PR's diff.
| @@ -256,7 +256,7 @@ export function useFireproof(name: string | Ledger = "useFireproof", config: Con | |||
|
|
|||
| const refreshRows = useCallback(async () => { | |||
| const res = await ledger.query<K, T, R>(mapFn, query); | |||
| setResult({ ...res, docs: res.rows.map((r) => r.doc as DocWithId<T>) }); | |||
| setResult({ ...res, docs: res.rows.map((r) => r.doc!) }); | |||
There was a problem hiding this comment.
Avoid using non-null assertions.
The non-null assertion (!) is used to tell TypeScript that r.doc will never be null. This can mask potential runtime errors if the assumption is incorrect.
Consider using proper type narrowing or providing a fallback:
- setResult({ ...res, docs: res.rows.map((r) => r.doc!) });
+ setResult({ ...res, docs: res.rows.map((r) => r.doc || {} as DocWithId<T>) });📝 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.
| setResult({ ...res, docs: res.rows.map((r) => r.doc!) }); | |
| setResult({ ...res, docs: res.rows.map((r) => r.doc || {} as DocWithId<T>) }); |
🧰 Tools
🪛 eslint
[error] 259-259: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
| @@ -322,7 +322,7 @@ export class DataStoreImpl extends BaseStoreImpl implements DataStore { | |||
| if (res.isErr()) { | |||
| throw this.logger.Error().Err(res.Err()).Msg("got error from gateway.put").AsError(); | |||
| } | |||
| return res.Ok(); | |||
| res.Ok(); | |||
There was a problem hiding this comment.
💡 Codebase verification
Return value missing in save method
The save method declares Promise<Result<void>> as its return type but doesn't return the result from res.Ok(). This violates the method's contract and could cause issues for callers expecting a Result object.
🔗 Analysis chain
Verify the save method's behavior change.
The save method now calls res.Ok() without returning it. Please verify if this change is intentional and doesn't affect the method's contract.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the save method's return type is used in the codebase.
# Search for calls to save method
rg -A 5 'save\(' | grep -v 'async save'
Length of output: 7968
Error: No shell scripts found
Script:
#!/bin/bash
# Find the save method implementation in store.ts
ast-grep --pattern $'async save($$$) {
$$$
}' src/blockstore/store.ts
Length of output: 72
Script:
#!/bin/bash
# Find the save method implementation with context
rg -B 5 -A 10 'async save\(' src/blockstore/store.ts
Length of output: 2221
Script:
#!/bin/bash
# Get more context around the first save method implementation
rg -B 5 -A 20 'async save\(meta: DbMeta' src/blockstore/store.ts
Length of output: 1133
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
tsconfig.json
Outdated
| "coverage/", | ||
| "smoke/react/", | ||
| "src/missingTypes/lib.deno.d.ts" | ||
| ], | ||
| "typeRoots": ["ts-types", "node_modules/@types"], |
There was a problem hiding this comment.
💡 Codebase verification
Critical: Type definitions directory missing and files not migrated
The change to typeRoots is problematic because:
- The new
ts-typesdirectory does not exist - Type definitions are still in
src/missingTypes:- src/missingTypes/lib.deno.d.ts
- src/missingTypes/prolly-trees.d.ts
Either create the ts-types directory and migrate the files, or revert the typeRoots change to prevent TypeScript compilation errors.
🔗 Analysis chain
Verify type definitions migration.
The change from src/missingTypes to ts-types requires verification:
- Ensure all type definitions have been migrated from
src/missingTypes - Confirm the
ts-typesdirectory exists and is properly structured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify type definitions migration
# Check if ts-types directory exists and contains .d.ts files
echo "Checking ts-types directory..."
if [ -d "ts-types" ]; then
find ts-types -name "*.d.ts" -type f
else
echo "ts-types directory not found"
fi
# Check for any remaining .d.ts files in src/missingTypes
echo -e "\nChecking src/missingTypes directory..."
if [ -d "src/missingTypes" ]; then
find src/missingTypes -name "*.d.ts" -type f
else
echo "src/missingTypes directory not found"
fi
Length of output: 395
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code style issues found. Run Prettier with --write to fix.
mabels
force-pushed
the
mabels/better-ts-lint
branch
from
6c99ace to
0ee506c
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
src/runtime/gateways/file/node-filesystem.ts (1)
26-26:⚠️ Potential issueRemove unsafe type assertion pattern
The combination of optional chaining (
?.) with non-null assertion (!) creates an unsafe pattern that could lead to runtime errors ifthis.fsis undefined.Replace with a type-safe implementation:
- return this.fs?.readdir(path, options)!; + if (!this.fs) { + throw new Error('FileSystem not initialized. Call start() first.'); + } + return this.fs.readdir(path, options);Consider a broader refactor
This pattern appears throughout the class. Consider these architectural improvements:
- Add a private
assertInitialized()method to centralize the initialization check- Initialize
fsin the constructor or make it required- Use TypeScript's strict initialization checks
Example implementation:
private assertInitialized(): void { if (!this.fs) { throw new Error('FileSystem not initialized. Call start() first.'); } } async readdir(path: PathLike, options?: ObjectEncodingOptions): Promise<string[]> { this.assertInitialized(); return this.fs.readdir(path, options); }🧰 Tools
🪛 eslint
[error] 26-26: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 26-26: Optional chain expressions can return undefined by design - using a non-null assertion is unsafe and wrong.
(@typescript-eslint/no-non-null-asserted-optional-chain)
src/react/useFireproof.ts (1)
259-259:⚠️ Potential issueReplace non-null assertion with proper type handling
The non-null assertion operator (
!) bypasses TypeScript's type checking and could lead to runtime errors ifr.docis undefined.Consider using proper type narrowing:
- setResult({ ...res, docs: res.rows.map((r) => r.doc!) }); + setResult({ ...res, docs: res.rows.map((r) => r.doc).filter((doc): doc is DocWithId<T> => doc !== undefined) });🧰 Tools
🪛 eslint
[error] 259-259: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
src/runtime/gateways/indexdb/gateway.ts (1)
43-43:⚠️ Potential issueReplace non-null assertion with explicit null check.
Instead of using the non-null assertion operator, add an explicit null check for better type safety. This aligns with the PR's objective of avoiding type assertions.
- const version = ensureVersion(url).getParam(PARAM.VERSION)!; + const version = ensureVersion(url).getParam(PARAM.VERSION); + if (!version) { + throw new Error('Version parameter is required'); + }🧰 Tools
🪛 eslint
[error] 43-43: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
🧹 Nitpick comments (6)
src/react/useFireproof.ts (1)
Line range hint
259-299: Maintain consistent type-safety approach across all hooksWhile the changes in
useAllDocsanduseChangesimprove type safety by removing type assertions and leveraging TypeScript's type inference, the change inuseLiveQueryintroduces a non-null assertion that goes against this pattern. Consider applying the same type-safe approach consistently across all hooks.tests/fireproof/ledger.test.ts (1)
174-175: Consider keeping template literals for better readability.The changes from template literals (e.g.,
`id-${6}`) to string concatenation (e.g.,"id-6") make the code less readable. Template literals are the preferred way to handle string interpolation in modern TypeScript/JavaScript.- const ok6 = await db.put({ _id: `id-6`, hello: "block" }); + const ok6 = await db.put({ _id: `id-${6}`, hello: "block" }); - const ok7 = await db.del(`id-7`); + const ok7 = await db.del(`id-${7}`); - expect(rows3[numDocs].key).toBe(`id-7`); + expect(rows3[numDocs].key).toBe(`id-${7}`);Also applies to: 188-189, 193-193
setup.indexdb.ts (1)
1-1: Consider renaminggthisfor better clarity.The function name
gthisis not very descriptive. Consider renaming it to something more meaningful likegetGlobalThisorgetGlobalContext.version-copy-package.ts (1)
71-73: Consider adding runtime validation for parsed JSON.While type assertions are used, consider adding runtime validation to ensure the parsed JSON matches the PackageJson interface structure.
- const mainPackageJson = JSON.parse(await fs.readFile("package.json", "utf8")) as PackageJson; + const mainPackageJson = validatePackageJson(JSON.parse(await fs.readFile("package.json", "utf8"))); + + function validatePackageJson(json: unknown): PackageJson { + if (!json || typeof json !== 'object') throw new Error('Invalid package.json'); + if (!('version' in json) || typeof json.version !== 'string') throw new Error('Missing/invalid version'); + if (!('dependencies' in json) || typeof json.dependencies !== 'object') throw new Error('Missing/invalid dependencies'); + return json as PackageJson; + }tsup.config.ts (1)
2-2: Consider cleaning up commented code.There's a significant amount of commented-out code including:
- resolve import and its usage
- skipEsm and ourMultiformat variables
- resolve plugin configurations
If these are no longer needed, consider removing them to improve code maintainability.
Also applies to: 67-69, 110-112, 175-177, 201-204
package.json (1)
54-54: Consider aligning @types/semver version with semverWhile the addition of
@types/semveris good, consider updating it to match the exact major.minor version ofsemver(7.6):- "@types/semver": "^7.5.8", + "@types/semver": "^7.6.0",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
.prettierignore(1 hunks)eslint.config.mjs(2 hunks)package.json(1 hunks)publish-package.ts(2 hunks)setup.indexdb.ts(1 hunks)src/blockstore/commit-queue.ts(2 hunks)src/blockstore/loader.ts(2 hunks)src/blockstore/store.ts(10 hunks)src/blockstore/transaction.ts(1 hunks)src/blockstore/types.ts(2 hunks)src/crdt-clock.ts(1 hunks)src/crdt-helpers.ts(3 hunks)src/crdt.ts(1 hunks)src/ledger.ts(6 hunks)src/react/useFireproof.ts(3 hunks)src/runtime/files.ts(1 hunks)src/runtime/gateways/file/node-filesystem.ts(1 hunks)src/runtime/gateways/fp-envelope-serialize.ts(1 hunks)src/runtime/gateways/indexdb/gateway.ts(3 hunks)src/runtime/key-bag-indexdb.ts(2 hunks)src/runtime/keyed-crypto.ts(2 hunks)src/runtime/wait-pr-multiformats/block.ts(3 hunks)src/version.ts(1 hunks)tests/fireproof/all-gateway.test.ts(4 hunks)tests/fireproof/fireproof.test.ts(2 hunks)tests/fireproof/ledger.test.ts(7 hunks)tests/fireproof/utils.test.ts(1 hunks)tests/gateway/file/loader-config.test.ts(13 hunks)tests/gateway/indexdb/loader-config.test.ts(4 hunks)tests/react/useFireproof.test.tsx(1 hunks)tsconfig.json(1 hunks)tsup.config.ts(7 hunks)version-copy-package.ts(3 hunks)vitest.file.config.ts(1 hunks)vitest.indexdb.config.ts(1 hunks)vitest.memory.config.ts(1 hunks)vitest.workspace.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .prettierignore
- vitest.workspace.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- src/version.ts
- src/blockstore/transaction.ts
- tests/fireproof/utils.test.ts
- src/runtime/files.ts
- src/crdt-clock.ts
- tests/gateway/file/loader-config.test.ts
- src/blockstore/commit-queue.ts
- tests/gateway/indexdb/loader-config.test.ts
- tests/fireproof/all-gateway.test.ts
- src/crdt.ts
- tests/fireproof/fireproof.test.ts
- tests/react/useFireproof.test.tsx
- src/blockstore/loader.ts
- src/blockstore/store.ts
- src/runtime/gateways/fp-envelope-serialize.ts
- src/crdt-helpers.ts
- tsconfig.json
- src/runtime/keyed-crypto.ts
- src/blockstore/types.ts
- src/runtime/key-bag-indexdb.ts
- src/ledger.ts
- src/runtime/wait-pr-multiformats/block.ts
🧰 Additional context used
🪛 GitHub Actions: CI
package.json
[error] Parsing error: "parserOptions.project" has been provided for @typescript-eslint/parser. The file was not found in any of the provided project(s)
🪛 eslint
src/react/useFireproof.ts
[error] 259-259: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
src/runtime/gateways/file/node-filesystem.ts
[error] 26-26: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 26-26: Optional chain expressions can return undefined by design - using a non-null assertion is unsafe and wrong.
(@typescript-eslint/no-non-null-asserted-optional-chain)
src/runtime/gateways/indexdb/gateway.ts
[error] 43-43: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
tests/fireproof/ledger.test.ts
[error] 463-463: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 474-474: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 485-485: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 504-504: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 521-521: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
🔇 Additional comments (14)
src/react/useFireproof.ts (2)
279-279: LGTM! Good removal of type assertionThe removal of the type assertion is a positive change as it lets TypeScript handle type inference naturally, which aligns with the PR objectives.
299-299: LGTM! Good removal of type assertionThe removal of the type assertion is a positive change as it lets TypeScript handle type inference naturally, which aligns with the PR objectives.
src/runtime/gateways/indexdb/gateway.ts (1)
16-16: LGTM! Good type simplification.Removing the redundant
<unknown>type parameter makes the code cleaner while maintaining type safety.setup.indexdb.ts (1)
1-1: LGTM! Good use of type annotation.The
Record<string | symbol, unknown>return type accurately represents the shape ofglobalThisand usesunknowninstead ofanyfor better type safety.publish-package.ts (3)
17-21: LGTM! Good use of optional chaining and nullish coalescing.The changes improve null safety by:
- Using optional chaining (
?.) to safely accessGITHUB_REF- Using nullish coalescing (
??) to provide a default empty string
27-27: LGTM! Good use of toString() for type safety.Converting prerelease items to strings before checking for "dev" is safer as they could be numbers.
6-6: Verify if explicit process import is necessary.Node.js automatically injects the
processglobal. The explicit import might be unnecessary unless it's required for type information.✅ Verification successful
The process import is necessary and correctly used
The import is required as the code actively uses multiple process features:
- Command line arguments (process.argv)
- Environment variables (process.env)
- Program termination (process.exit)
- Output streaming (process.stdout)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if process is used as a type anywhere in the file ast-grep --pattern 'process: $_'Length of output: 77
Script:
#!/bin/bash # Search for process usage in publish-package.ts rg "process\." publish-package.ts -A 2Length of output: 707
version-copy-package.ts (2)
51-55: LGTM! Good addition of PackageJson interface.The interface properly defines the expected shape of package.json objects and improves type safety.
65-66: LGTM! Good improvement in file system check logic.Storing the stat result before checking isDirectory is more efficient and clearer.
tsup.config.ts (2)
5-6: LGTM! Good type annotations for arrays.Explicit type annotations for
nodeInternalsandwebInternalsimprove code clarity.
102-102: LGTM! Good use of nullish coalescing for externals.Using
??instead of||for external arrays provides better handling of undefined values.Also applies to: 163-163, 185-185
eslint.config.mjs (3)
7-11: LGTM! Enhanced type checking configurationThe transition to type-checked configurations (
strictTypeChecked,stylisticTypeChecked,recommendedTypeChecked) is a good improvement that enables more sophisticated type checking capabilities.
24-38: LGTM! Comprehensive ignore patternsThe updated ignore patterns appropriately exclude test configurations, build artifacts, and development files from linting.
57-62: LGTM! Good practice enforcing immutabilityThe addition of
@typescript-eslint/prefer-readonlyrule promotes immutability, which is a TypeScript best practice.Let's verify the impact of this rule:
✅ Verification successful
Enabling
@typescript-eslint/prefer-readonlyrule is safeThe codebase already follows good practices with readonly properties where possible. The few mutable properties found are justified by their implementation requirements (e.g., queue management, concurrency control).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for existing mutable class properties that would be affected by this rule ast-grep --pattern 'class $_ { $$$ private $_: $_ $$$ }'Length of output: 3453
| @@ -91,7 +91,7 @@ export class IndexDBGateway implements Gateway { | |||
| this.logger = ensureLogger(sthis, "IndexDBGateway"); | |||
| this.sthis = sthis; | |||
| } | |||
| _db: IDBPDatabase<unknown> = {} as IDBPDatabase<unknown>; | |||
| _db: IDBPDatabase = {} as IDBPDatabase; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove type assertion and initialize properly.
The type assertion as IDBPDatabase with an empty object is unsafe and goes against the PR's objective of avoiding type casts. Consider proper initialization:
- _db: IDBPDatabase = {} as IDBPDatabase;
+ private _db?: IDBPDatabase;Then add null checks in methods that use _db:
if (!this._db) {
throw new Error('Database not initialized. Call start() first.');
}
tests/fireproof/ledger.test.ts
Outdated
| @@ -460,7 +460,7 @@ describe("ledger with files input", () => { | |||
|
|
|||
| it("Should fetch the images", async function () { | |||
| const doc = await db.get(result.id); | |||
| const files = doc._files as DocFiles; | |||
| const files = doc._files!; | |||
There was a problem hiding this comment.
Replace non-null assertions with type guards.
The use of non-null assertions (!) is flagged by ESLint and goes against the PR's objective of avoiding type casts. Consider using type guards to handle potential null values more safely.
Here's how to refactor the code:
- const files = doc._files!;
+ if (!doc._files) {
+ throw new Error('Expected _files to be defined');
+ }
+ const files = doc._files;
- let file = (await fileMeta.file?.())!;
+ const fileResult = await fileMeta.file?.();
+ if (!fileResult) {
+ throw new Error('Expected file to be defined');
+ }
+ let file = fileResult;Apply similar changes to all instances of non-null assertions in the file.
Also applies to: 474-474, 485-485, 504-504, 521-521
🧰 Tools
🪛 eslint
[error] 463-463: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
vitest.file.config.ts
Outdated
| @@ -1,9 +1,9 @@ | |||
| import { defineConfig } from "vitest/config"; | |||
| import { defineConfig, Plugin } from "vitest/config"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary type assertion
The type assertion as Plugin contradicts the PR objective of avoiding type casts. Since tsconfigPaths() returns a valid Vite plugin that's compatible with Vitest, the type assertion is unnecessary.
Apply this diff to remove the type assertion:
-import { defineConfig, Plugin } from "vitest/config";
+import { defineConfig } from "vitest/config";
import tsconfigPaths from "vite-tsconfig-paths";
export default defineConfig({
- plugins: [tsconfigPaths() as Plugin],
+ plugins: [tsconfigPaths()],Also applies to: 6-6
vitest.indexdb.config.ts
Outdated
| // import { defineConfig } from "vite"; | ||
| import { defineConfig, Plugin } from "vitest/config"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up imports and remove type assertion
Good change on using vitest/config instead of vite. However, the type assertion as Plugin is unnecessary and contradicts the PR objective of avoiding type casts.
Apply this diff to clean up the code:
-// import { defineConfig } from "vite";
-import { defineConfig, Plugin } from "vitest/config";
+import { defineConfig } from "vitest/config";
import tsconfigPaths from "vite-tsconfig-paths";
export default defineConfig({
- plugins: [tsconfigPaths() as Plugin],
+ plugins: [tsconfigPaths()],Also applies to: 7-7
vitest.memory.config.ts
Outdated
| @@ -1,9 +1,9 @@ | |||
| import { defineConfig } from "vitest/config"; | |||
| import { defineConfig, Plugin } from "vitest/config"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary type assertion for consistency
Similar to the other configuration files, the type assertion as Plugin is unnecessary here and should be removed to align with the PR objective of avoiding type casts.
Apply this diff to remove the type assertion:
-import { defineConfig, Plugin } from "vitest/config";
+import { defineConfig } from "vitest/config";
import tsconfigPaths from "vite-tsconfig-paths";
export default defineConfig({
- plugins: [tsconfigPaths() as Plugin],
+ plugins: [tsconfigPaths()],Also applies to: 6-6
eslint.config.mjs
Outdated
| languageOptions: { | ||
| parserOptions: { | ||
| sourceType: "module", | ||
| project: "./tsconfig.json", | ||
| // projectService: true, | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix the TypeScript project configuration
The ESLint pipeline is failing because it cannot locate the TypeScript configuration file. This needs to be addressed for the type-checking to work correctly.
Try these solutions:
- Verify that
tsconfig.jsonexists in the root directory - Consider using an absolute path or multiple possible locations:
- project: "./tsconfig.json",
+ project: [
+ "./tsconfig.json",
+ "./packages/*/tsconfig.json"
+ ],📝 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.
| languageOptions: { | |
| parserOptions: { | |
| sourceType: "module", | |
| project: "./tsconfig.json", | |
| // projectService: true, | |
| tsconfigRootDir: import.meta.dirname, | |
| }, | |
| }, | |
| languageOptions: { | |
| parserOptions: { | |
| sourceType: "module", | |
| project: [ | |
| "./tsconfig.json", | |
| "./packages/*/tsconfig.json" | |
| ], | |
| // projectService: true, | |
| tsconfigRootDir: import.meta.dirname, | |
| }, | |
| }, |
@jchris if you want help
here is something!
It's a lot of editing, but it should not be hard. pls do not use any casts or ignore comments.
If something is not clear how to resolve -> ask.
Summary by CodeRabbit
Release Notes
Overview
This release includes multiple improvements and refinements across the Fireproof codebase, focusing on type safety, error handling, and code clarity.
Key Changes
Type Safety Enhancements
Configuration Updates
node_modulessemverCode Refinement
Testing
Development Dependencies
semverCompatibility