[WIP] Replace unmaintained packages#750
Open
bennycode wants to merge 4 commits into
Open
MacroscopeApp / Review for correctness
succeeded
Oct 24, 2025 in 4m 28s
No issues identified (42 code objects reviewed).
• Merge Base:
3788527
• Head:746a4d9
Details
| ✅ | File Path | Comments Posted | Issues |
|---|---|---|---|
| ✅ | example/src/hooks.tsx |
0 | 19 evaluated, 18 filtered |
| ✅ | example/src/tests/clientTests.ts |
0 | 15 evaluated, 15 filtered |
| ✅ | example/src/tests/conversationTests.ts |
0 | 3 evaluated, 3 filtered |
| ✅ | example/src/tests/fileSystemHelpers.ts |
0 | 2 evaluated, 2 filtered |
| ✅ | example/src/tests/groupPerformanceTests.ts |
0 | 2 evaluated, 2 filtered |
| ✅ | example/src/tests/historySyncTests.ts |
0 | 5 evaluated, 5 filtered |
Filtered Issues Details
example/src/hooks.tsx
- line 760: The hook's returned function types declare
save: (address: string) => voidandclear: () => void, but the implementations areasyncfunctions returningPromise<void>. This contract mismatch can cause unhandled promise rejections if callers treat them as synchronous (as the types suggest). For example, ifSecureStore.setItemAsyncorrefetchrejects, the returned promise will reject, but a caller expectingvoidwill notcatchit, leading to unhandled rejection. This violates contract parity and makes error handling inconsistent. [ Out of scope ] - line 763: Errors from the
useQueryare silently hidden: the hook returns onlyaddresswithout exposingstatus,error, or any fallback handling. If the storage call fails (e.g., due to the incorrectSecureStore.getItemusage or other runtime errors), consumers cannot distinguish between "no saved address" and a failed query. This silent failure path violates the requirement to provide a defined and visible outcome for error inputs. [ Low confidence ] - line 765: The query function calls
SecureStore.getItem('xmtp.address'), butexpo-secure-storeprovidesgetItemAsync.SecureStore.getItemis undefined and will throwTypeError: SecureStore.getItem is not a functionwhen the query runs (both initial load and onrefetch). This causes theuseQueryto error and preventsaddressfrom ever resolving. [ Already posted ] - line 787:
SecureStore.getItemis called instead of the correct async APISecureStore.getItemAsync, and it is not awaited. In Expo SecureStore, the method isgetItemAsync(key)and returns a Promise. CallingSecureStore.getItem(key)will throw at runtime (TypeError: SecureStore.getItem is not a function). Even if a similarly named function existed, failing toawaitthe Promise would cause downstream logic to treat a Promise as a string, breaking control flow. [ Already posted ] - line 796:
crypto.getRandomValues(randomBytes)may not be available in the React Native/Expo runtime without an explicit polyfill (e.g.,react-native-get-random-values) or usingexpo-random. Ifcryptois undefined or lacksgetRandomValues, this will throw at runtime when generating the key. [ Low confidence ] - line 806: Because the result of
SecureStore.getItemis not awaited and the wrong method is used,resultis not a string and downstream logic attempts to convert it withhexStringToUint8Array(result). Passing a non-string (or a Promise) intohexStringToUint8Arraywill causehexString.lengthto throw a runtime error, or produce incorrect behavior if coerced. [ Already posted ] - line 834: The helper
ensureFileUriblindly prefixes withfile://when the path does not start withfile://. If a valid non-file URI (e.g.,content://on Android) is passed, this produces an invalid URI likefile://content://..., causingexpo-file-systemoperations to fail. The function should detect and preserve other URI schemes or validate input. [ Low confidence ] - line 835:
ensureFileUriblindly prependsfile://to any non-file://input, corrupting non-file URIs. For example, inputs likecontent://...,asset://...,http://..., ordata:...will becomefile://content://...which is an invalid URI and will likely causeFileSystem.getInfoAsyncto throw. This yields false negatives infileExistsand masks real errors. [ Already posted ] - line 835: No percent-encoding of path segments is performed. Inputs containing spaces or reserved characters (e.g.,
/dir/My File (final).txt) are returned asfile:////dir/My File (final).txt, which is not a strictly valid URI and may be rejected by some consumers. This can causegetInfoAsyncto throw or misinterpret the path. [ Low confidence ] - line 835:
ensureFileUridoes not handle empty or whitespace-only paths. For an empty string, it returns"file://", which is not a valid file URI and will likely causeFileSystem.getInfoAsyncto throw. The caller then returnsfalse, masking input errors and potentially hiding bugs upstream. [ Low confidence ] - line 835: The check
path.startsWith('file://')is case-sensitive, but URI schemes are case-insensitive. Inputs likeFILE://...orFile://...(which are valid forms of thefilescheme) will be incorrectly treated as non-file and rewritten tofile://FILE://..., producing an invalid URI. [ Low confidence ] - line 835: Relative paths are turned into authority-based URIs. For example,
"foo/bar"becomes"file://foo/bar", which is parsed as hostfooand path/bar, not a local file path. This likely breaksgetInfoAsyncand causes false negatives instead of either resolving relative to a known base or rejecting the input. [ Low confidence ] - line 835: Windows paths are not normalized. An input like
"C:\\foo\\bar"becomes"file://C:\\foo\\bar", which is not a valid file URI (wrong separators and missing third slashfile:///C:/...). This will likely fail in any URI consumer and cause false negatives. [ Low confidence ] - line 835: The function does not recognize the alternate but still-seen form
"file:/path"(single slash after scheme). Such inputs will be rewritten to"file://file:/path", producing an invalid URI rather than either accepting or normalizing them. [ Already posted ] - line 853: In
getFileSize, the code returnsinfo.sizewheninfo.existsis true, butinfo.sizecan beundefineddepending on platform or file type. Returningundefinedfrom a function declared to returnPromise<number>leads to downstream numeric operations producingNaN(e.g., when adding tofileSizeTotal). A safe fallback (e.g., returning0whensizeis not a number) is needed. [ Low confidence ] - line 870: The code uses
Bufferin React Native/Expo (Buffer.from(fileContent, 'base64')andBuffer.from(digest)), butBufferis not guaranteed to be defined in Expo without a polyfill. In environments lacking a globalBuffer, this will throwReferenceError: Buffer is not defined, breaking file digest calculation. [ Low confidence ] - line 873: The function
calculateFileDigestusesExpoCrypto.digestwith aBufferinput (buffer), but Expo'sexpo-cryptoAPI most commonly exposesdigestStringAsyncfor hashing strings and may not provide adigestmethod that accepts a NodeBuffer(or may expect anArrayBuffer). IfExpoCrypto.digestis undefined or rejects the input type at runtime, this will throw (TypeError/invalid input), breaking digest calculation and attachment verification. [ Low confidence ] - line 877: Potential incorrect hex conversion: If
ExpoCrypto.digestreturns a hex string (as someexpo-cryptoAPIs do), the codereturn Buffer.from(digest).toString('hex')will return the hex of the UTF-8 bytes of the hex string, yielding an incorrect digest. This would cause content digest verification to fail even for matching files. [ Low confidence ]
example/src/tests/clientTests.ts
- line 24: Top-level initialization
const DOCUMENT_DIRECTORY = documentDirectoryPath()callsensureDocumentDirectory()at module import time. IfFileSystem.documentDirectoryis unavailable in the runtime (e.g., non-Expo React Native environment),ensureDocumentDirectory()throws, causing the entire test module to crash during import. This makes every test in the module fail before any test logic runs and introduces a hard runtime dependency on Expo'sFileSystem.documentDirectorybeing present. [ Low confidence ] - line 98: Possible out-of-bounds access on
states[0]without verifying thatstatescontains at least one element. IfClient.inboxStatesForInboxIds('local', [inboxId])returns an empty array,states[0]will beundefinedand accessing.installationswill throw at runtime before any assertion is reached. Add an explicit length check (or assert) before dereferencingstates[0]. [ Test / Mock code ] - line 98: Misleading assertion message:
assert(states[0].installations.length === 2, 'should equal 5 installations'). The assertion verifies length equals 2, but the error message claims it should equal 5. This contradiction will mislead investigation if the test fails. [ Test / Mock code ] - line 127: Resource cleanup is not ensured on failure paths. Multiple asynchronous operations and assertions occur before calling
dropLocalDatabaseConnection()anddeleteLocalDatabase()for both clients. If any await or assertion throws earlier, these cleanups will not execute, potentially leaving database connections/files open. Wrap the critical section in a try/finally to guarantee cleanup on all exit paths. [ Test / Mock code ] - line 167: Possible out-of-bounds access on
states[0]without verifying thatstatescontains at least one element. IfClient.inboxStatesForInboxIds('local', [inboxId])returns an empty array,states[0]will beundefinedand accessing.installationswill throw at runtime. Add an explicit length check (or assert) before dereferencingstates[0]. [ Test / Mock code ] - line 188: Resource cleanup is not ensured on failure paths. Multiple asynchronous operations and assertions occur before calling
dropLocalDatabaseConnection()anddeleteLocalDatabase()for both clients. If any await or assertion throws earlier, these cleanups will not execute, potentially leaving database connections/files open. Use a try/finally to guarantee cleanup. [ Test / Mock code ] - line 253: In the installation limit test, the directory for the 11th client is used without prior creation:
dbDirectory:${basePath}_11``. Earlier code ensures directories only for indices 0..10. IfClient.createrequires the directory to exist (consistent with prior explicit directory creation logic), this can throw at runtime when creating `sixthNow`. It also risks leaving inconsistent state and makes cleanup assumptions fragile. [ Out of scope ] - line 928: The test relies on
joinDocumentPath(...), which callsdocumentDirectoryPath()and ultimatelyensureDocumentDirectory()that throws ifFileSystem.documentDirectoryis unavailable. In environments where ExpoFileSystem.documentDirectoryis undefined (e.g., non-Expo environments or incorrect initialization), this test will crash before creating directories. Consider guarding for availability or skipping the test whendocumentDirectoryis not provided. [ Low confidence ] - line 970: The signer created by
adaptEthersWalletToSignerusesdebugLoginsidesignMessage, butdebugLogis not defined anywhere. WhenalixSigner.signMessage(...)is invoked here, it will throw aReferenceErrorat runtime before attempting to sign. This occurs for both sign operations in this test. Fix by either definingdebugLog, removing the calls, or guarding them. [ Low confidence ] - line 1055: The test uses
joinDocumentPath(...)which depends on ExpoFileSystem.documentDirectory. IfdocumentDirectoryis undefined in the runtime environment,ensureDocumentDirectory()throws, causing the test to fail before any client creation. This environment dependency was introduced by switching fromreact-native-fspaths to Expo helpers. Guard the test or ensure the environment providesdocumentDirectory. [ Low confidence ] - line 1094: The signer created by
adaptEthersWalletToSignerusesdebugLoginsidesignMessage, butdebugLogis not defined. Duringalix3.revokeInstallations(adaptEthersWalletToSigner(alixWallet), ...), the SDK will callsignMessageon the provided signer, causing aReferenceErrorat runtime. DefinedebugLog, remove the calls, or guard them. [ Low confidence ] - line 1195: The archive test uses optional chaining
boGroup?.send('hey')without verifying thatboGroupexists. IffindConversation(group.id)returnsundefineddue to sync timing or propagation delays, the send is silently skipped, but subsequent assertions assume the message was sent and expectmessages.length === 3. This leads to a confusing failure later rather than an immediate, clear error when the conversation is not found. [ Out of scope ] - line 1212: The archive test does not clean up created clients or delete local databases/directories at the end of the test.
alixandalix2remain with active databases and potentially open connections. This can leak resources and leave residual state that may affect subsequent tests or retries. [ Out of scope ] - line 1288:
crypto.getRandomValuesis used to create the encryption key for archives and databases. In some React Native environments,cryptois not defined unless a polyfill (e.g.,react-native-get-random-values) is installed and initialized. If unavailable, this will throw at runtime during test setup. There is no guard or fallback. This can cause test runs to crash in environments lacking the polyfill. [ Test / Mock code ] - line 1292: ensureDirectory/pathExists rely on
toUri(path)fromfileSystemHelperswhich prefixesfile://to whatever string is passed. Whenpathcomes fromjoinDocumentPath(...), it is an absolute path starting with/. This produces a malformed URI likefile:////...(four slashes) instead of the canonicalfile:///.... Passing such a URI toexpo-file-systemmethods likegetInfoAsyncandmakeDirectoryAsynccan lead to failures or false negatives, causing directory existence checks to fail and directories not to be created as expected. This regression was introduced by replacing RNFS paths with the new helpers. The faulty usage occurs when callingensureDirectory(dbPath). [ Low confidence ]
example/src/tests/conversationTests.ts
- line 150: Using
joinDocumentPath(...)introduces a hard dependency onexpo-file-system'sdocumentDirectory. IfFileSystem.documentDirectoryis unavailable (e.g., running tests in a non-Expo environment or on platforms where the directory is not initialized),documentDirectoryPath()throws, causing the entire test to fail during setup. Previously, the code usedRNFS.DocumentDirectoryPath, which may be available in non-Expo RN environments. This change narrows compatibility without a guard or fallback. [ Test / Mock code ] - line 153: Same malformed URI risk as above when checking/creating database directories in
conversationTests: usingpathExists(dbDirPath)and thenensureDirectory(dbDirPath)withdbDirPathbuilt byjoinDocumentPath(...)will producefile:////...URIs under the hood, potentially causingexpo-file-systemto misinterpret the path. This risks false negatives on existence checks and failure to create the directory. [ Low confidence ] - line 156: The existence and directory creation checks assume that a path existing means a directory exists.
pathExistsreturnsinfo.existswithout checkinginfo.isDirectory, andensureDirectoryonly checks!info.existsbefore creating. If a regular file exists at the desired path, the code will incorrectly treat it as a directory and skip creation, leading to later failures when passingdbDirectoryto the SDK or writing archives. This should explicitly validate that the path is a directory and either create it or throw a clear error if a non-directory exists at that path. [ Low confidence ]
example/src/tests/fileSystemHelpers.ts
- line 29:
pathExistsreturnsinfo.existswithout distinguishing between files and directories. The tests usepathExiststo decide whether to create a directory. If a file exists at the target path,pathExistswill returntrueand the code will skip directory creation, leaving a file where a directory is expected. Subsequent operations using that path as a directory may fail. Consider returning both existence andisDirectory, or handle the directory-vs-file distinction at the call site. [ Low confidence ] - line 34:
ensureDirectorydoes not verify that an existing path is a directory. If a file exists atpath,FileSystem.getInfoAsyncwill returnexists: true, and the function will skipmakeDirectoryAsync. Callers that expectpathto be a directory (e.g., passing it asdbDirectory) may subsequently fail at runtime because they are actually pointing at a file. The function should checkinfo.isDirectoryand either create the directory (if possible) or throw an error when a non-directory path exists. [ Low confidence ]
example/src/tests/groupPerformanceTests.ts
- line 39: The code assumes that
pathExists(dbDirPath)reliably indicates that a directory exists atdbDirPath. IfpathExistsreturnstruefor any filesystem entry (file or directory), this can cause the code to skipensureDirectory(dbDirPath)when a file exists at that path. Subsequent operations that expect a directory fordbDirectorymay fail at runtime (e.g., when the client attempts to create files within that path). To avoid this, either check that the path is specifically a directory before skipping creation, or callensureDirectoryunconditionally. IfpathExistsdoes not distinguish directories, it should be replaced with a function that checksisDirectoryor the code should fetchFileSystem.getInfoAsyncand verifyinfo.isDirectory === true. [ Low confidence ] - line 96: The performance assertions compare measured durations across separate runs (
create,build, andbuild with inboxId) and assumebuildis always faster thancreateandbuild with inboxIdis faster thanbuild. These measurements are susceptible to environmental fluctuations (JIT warm-up, I/O variability, device load), making the test potentially flaky. A temporary slowdown or GC pause could cause test failures unrelated to actual regressions. Consider adding tolerances, warming up operations, averaging multiple runs, or converting these to non-failing metrics reporting rather than hard assertions. [ Test / Mock code ]
example/src/tests/historySyncTests.ts
- line 85: Possible undefined dereference:
dm2is obtained viaawait alix2.conversations.findConversation(dm.id), which can returnundefined. The code then usesdm2!(non-null assertion) atnew ConsentRecord(dm2!.id, 'conversation_id', 'allowed'). Ifdm2isundefineddue to timing or sync delays, accessingdm2.idwill throw a runtimeTypeError. Guard againstundefinedbefore dereferencing or assert/await until the conversation is present. [ Out of scope ] - line 88: Possible undefined dereference:
dm2can beundefinedfromfindConversation. The code usesdm2!again atawait alix2.preferences.conversationConsentState(dm2!.id). Ifdm2isundefined, accessing.idwill throw. Add a guard or ensure the conversation exists before dereferencing. [ Out of scope ] - line 147: Possible undefined dereference:
alix2Groupis obtained viaawait alix2.conversations.findConversation(alixGroup.id), which can returnundefined. The code then usesalix2Group!atawait alix2Group!.updateConsent('denied'). Ifalix2Groupisundefineddue to timing or sync delays, this will throw a runtimeTypeError. Guard forundefinedor ensure the group conversation exists before dereferencing. [ Out of scope ] - line 149: Possible undefined dereference:
dmis created byawait alix2.conversations.newConversation(bo.inboxId). IfnewConversationreturnsundefinedornulldue to an error or precondition, the non-null assertiondm!atawait dm!.updateConsent('denied')will cause a runtime failure. Add a guard to verifydmis defined before invoking methods. [ Out of scope ] - line 217: Incorrect cleanup API: The test starts a preferences stream via
alix.preferences.streamPreferenceUpdates(...)but later callsalix.preferences.cancelStreamConsent(). This mismatched cancellation likely leaves the preference updates stream running, causing resource/callback leaks and violating single paired cleanup. Use the corresponding cancellation method for preference updates (e.g.,cancelStreamPreferenceUpdates()or the API appropriate to the started stream). [ Out of scope ]
Loading