Fix React Context providers, restore missing exports, and uncomment examples#29
Conversation
Co-authored-by: miccy <9729864+miccy@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
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: 5
🤖 Fix all issues with AI agents
In `@apps/web/src/app/`(playgrounds)/playgrounds/minimal/EvoluMinimalExample.tsx:
- Around line 242-248: The restore/reset handlers currently do nothing because
evolu.restoreAppOwner and evolu.resetAppOwner are commented out in
handleRestoreAppOwnerClick and handleResetAppOwnerClick; either uncomment those
calls to re-enable the buttons (ensuring proper await/error handling) or remove
the buttons from the UI, or add a visible disabled state/tooltip/toast in the
handlers explaining the feature is intentionally disabled; locate the handlers
by name (handleRestoreAppOwnerClick, handleResetAppOwnerClick) and update the
code accordingly to implement one of these three options.
In `@examples/react-expo/app/index.tsx`:
- Around line 133-553: Move the inline components Todos, TodoItem, OwnerActions,
AuthActions, OwnerProfile, and CustomButton out of the EvoluDemo function so
they are declared at module scope; update their signatures to accept required
props (e.g., Todos receives todos (from useQuery(todosQuery)) and insert or a
handler, TodoItem receives row and update, OwnerActions receives
evolu/appOwner/authResult/localAuth/service/ownerIds or callbacks, AuthActions
receives evolu/localAuth/service/ownerIds/authResult, OwnerProfile receives
ownerId/username/handleLoginPress, CustomButton receives
title/onPress/variant/style) and wire them where used by passing those values
(or memoized callbacks from useEvolu/useQuery) so that useEvolu/useQuery remain
inside EvoluDemo and the components are stable across renders.
- Around line 47-82: The cleanup returned from the async IIFE inside useEffect
is a Promise and never runs; fix by making the effect return a synchronous
cleanup that calls the unsubscribe produced by evolu.subscribeError: after
creating Evolu via Evolu.createEvolu(...) assign the result of
evolu.subscribeError(...) to a local variable (e.g., let unsubscribe) and then
have the outer useEffect return a synchronous function that calls
unsubscribe?.(); ensure Evolu.createEvolu, evolu.subscribeError, setEvolu, and
the async IIFE remain but that the actual unsubscribe is captured and invoked
from the effect's direct return so it will run on unmount.
- Around line 630-636: The style object todoItem contains an invalid negative
padding value (paddingHorizontal: -8) which should be non-negative; remove or
set paddingHorizontal to 0 and rely on the existing negative marginHorizontal
(or change marginHorizontal if the goal is to visually extend the item) so
update the todoItem style in index.tsx (todoItem) to eliminate the negative
padding and use negative margin or parent padding adjustments instead.
In `@packages/react-native/src/exports/expo-op-sqlite.ts`:
- Around line 9-15: Uncomment the implementation in createExpoDeps.ts so the
createExpoDeps symbol is actually exported (it’s currently fully commented out),
then ensure the exported API matches what's imported in expo-op-sqlite.ts
(createExpoDeps with properties evoluReactNativeDeps and localAuth);
specifically restore the function or const definition that returns/create those
properties and export it (named export createExpoDeps) so the import in
expo-op-sqlite.ts resolves without TS2306.
🧹 Nitpick comments (8)
packages/react/src/createUseEvolu.ts (1)
4-12: JSDoc example references potentially unavailable methods.The example shows
const { insert, update } = useEvolu();but per the Evolu interface snippets, mutation methods likeinsertandupdateappear to be commented out. This could confuse consumers if the API isn't available yet.💡 Suggested fix: Update example to show available methods
* ### Example * * ```ts * const useEvolu = createUseEvolu(evolu); -* const { insert, update } = useEvolu(); +* const evolu = useEvolu(); +* const rows = evolu.getQueryRows(query); * ```apps/web/src/app/(playgrounds)/playgrounds/full/EvoluFullExample.tsx (2)
131-138: Consider structured error handling overalert().While acceptable for a playground demo, using
alert()blocks the main thread and provides poor UX. For a "full" example that may serve as a reference implementation, consider using a toast notification or error boundary pattern.
654-660: Commented-out functionality may confuse users.The restore and reset owner methods are commented out, making these buttons non-functional. Consider either implementing stubs that show "not yet available" feedback, or hiding/disabling these buttons until the API is ready.
examples/react-expo/app/index.tsx (3)
2-2: Consider using named imports instead of namespace import.Per coding guidelines, namespace imports (
import * as Evolu) should be avoided in favor of named imports. However, for example/demo code this pattern can improve readability by making Evolu API usage explicit.
108-128: Query and hook factory created on every render.
createUseEvoluandcreateQueryBuilderare called inside the component body, creating new instances on every render. While this may not cause functional issues due to internal memoization, it's inefficient.Consider memoizing these or moving them outside the component if the
evoluprop is stable.♻️ Proposed optimization using useMemo
const EvoluDemo = ({ evolu, ownerIds, authResult, }: { evolu: Evolu.Evolu<typeof Schema>; ownerIds: Array<Evolu.AuthList> | null; authResult: Evolu.AuthResult | null; }): React.ReactNode => { - const useEvolu = createUseEvolu(evolu); - - // Create a query builder (once per schema). - const createQuery = Evolu.createQueryBuilder(Schema); - - // Evolu uses Kysely for type-safe SQL (https://kysely.dev/). - const todosQuery = createQuery((db) => + const useEvolu = useMemo(() => createUseEvolu(evolu), [evolu]); + + const todosQuery = useMemo( + () => + Evolu.createQueryBuilder(Schema)((db) => db .selectFrom("todo") .select(["id", "title", "isCompleted"]) .where("isDeleted", "is not", Evolu.sqliteTrue) .where("title", "is not", null) .$narrowType<{ title: Evolu.kysely.NotNull }>() - .orderBy("createdAt"), + .orderBy("createdAt") + ), + [], );
567-577: Redundant nestedEvoluProvider.
EvoluProvideris already applied in the parentIndexcomponent (line 93). This nested provider at line 567 is redundant sinceevoluis passed as a prop toEvoluDemoand the components inside already access it viauseEvolu()which reads from the outer provider.♻️ Remove redundant provider
<View style={styles.maxWidthContainer}> <View style={styles.header}> <Text style={styles.title}>Minimal Todo App (Evolu + Expo)</Text> </View> - <EvoluProvider value={evolu}> - {/* - Suspense delivers great UX (no loading flickers) and DX (no loading - states to manage). Highly recommended with Evolu. - */} - <Suspense> - <Todos /> - <OwnerActions /> - <AuthActions /> - </Suspense> - </EvoluProvider> + {/* + Suspense delivers great UX (no loading flickers) and DX (no loading + states to manage). Highly recommended with Evolu. + */} + <Suspense> + <Todos /> + <OwnerActions /> + <AuthActions /> + </Suspense> </View>apps/web/src/app/(playgrounds)/playgrounds/minimal/EvoluMinimalExample.tsx (2)
3-3: Namespace import used for Evolu.Same as the React Native example - consider using named imports per coding guidelines. For playground/example code, namespace imports can be acceptable for clarity.
34-37: TODO comments indicate incomplete configuration.These comments suggest unfinished work regarding
reloadUrlconfiguration for web deps. Consider creating an issue to track this or removing the comments if not applicable to this PR.Would you like me to open an issue to track the
reloadUrlconfiguration decision?
apps/web/src/app/(playgrounds)/playgrounds/minimal/EvoluMinimalExample.tsx
Show resolved
Hide resolved
| todoItem: { | ||
| flexDirection: "row", | ||
| alignItems: "center", | ||
| paddingVertical: 8, | ||
| paddingHorizontal: -8, | ||
| marginHorizontal: -8, | ||
| }, |
There was a problem hiding this comment.
Invalid negative padding value.
paddingHorizontal: -8 at line 634 is invalid in React Native. Padding values must be non-negative. This will either be ignored or cause unexpected layout behavior. If the intent is to extend the item beyond its container, consider using negative margins or adjusting the parent padding instead.
🐛 Proposed fix
todoItem: {
flexDirection: "row",
alignItems: "center",
paddingVertical: 8,
- paddingHorizontal: -8,
- marginHorizontal: -8,
+ marginHorizontal: -8,
},📝 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.
| todoItem: { | |
| flexDirection: "row", | |
| alignItems: "center", | |
| paddingVertical: 8, | |
| paddingHorizontal: -8, | |
| marginHorizontal: -8, | |
| }, | |
| todoItem: { | |
| flexDirection: "row", | |
| alignItems: "center", | |
| paddingVertical: 8, | |
| marginHorizontal: -8, | |
| }, |
🤖 Prompt for AI Agents
In `@examples/react-expo/app/index.tsx` around lines 630 - 636, The style object
todoItem contains an invalid negative padding value (paddingHorizontal: -8)
which should be non-negative; remove or set paddingHorizontal to 0 and rely on
the existing negative marginHorizontal (or change marginHorizontal if the goal
is to visually extend the item) so update the todoItem style in index.tsx
(todoItem) to eliminate the negative padding and use negative margin or parent
padding adjustments instead.
| import { createExpoDeps } from "../createExpoDeps.js"; | ||
| import { createOpSqliteDriver } from "../sqlite-drivers/createOpSqliteDriver.js"; | ||
|
|
||
| // eslint-disable-next-line evolu/require-pure-annotation | ||
| export const { evoluReactNativeDeps, localAuth } = createExpoDeps({ | ||
| createSqliteDriver: createOpSqliteDriver, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking if createExpoDeps.ts exists ==="
fd "createExpoDeps.ts" packages/react-native/src/
echo -e "\n=== File contents ==="
cat packages/react-native/src/createExpoDeps.ts 2>/dev/null || echo "File not found at expected location"
echo -e "\n=== Searching for file in entire packages/react-native ==="
fd "createExpoDeps" packages/react-native/
echo -e "\n=== Checking the importing file ==="
cat -n packages/react-native/src/exports/expo-op-sqlite.tsRepository: SQLoot/evolu-plan-b
Length of output: 6397
Uncomment the implementation in createExpoDeps.ts.
The entire file content is commented out, resulting in no exports. The import statement in expo-op-sqlite.ts on line 9 attempts to import createExpoDeps, which is not exported because all code in createExpoDeps.ts is commented. This causes the TS2306 error: "File is not a module". Uncomment the implementation code to resolve the compilation failure.
🧰 Tools
🪛 GitHub Actions: CI
[error] 9-9: TS2306: File '/home/runner/work/evolu-plan-b/evolu-plan-b/packages/react-native/src/createExpoDeps.ts' is not a module.
🤖 Prompt for AI Agents
In `@packages/react-native/src/exports/expo-op-sqlite.ts` around lines 9 - 15,
Uncomment the implementation in createExpoDeps.ts so the createExpoDeps symbol
is actually exported (it’s currently fully commented out), then ensure the
exported API matches what's imported in expo-op-sqlite.ts (createExpoDeps with
properties evoluReactNativeDeps and localAuth); specifically restore the
function or const definition that returns/create those properties and export it
(named export createExpoDeps) so the import in expo-op-sqlite.ts resolves
without TS2306.
Addresses code review feedback on PR #28 which broke downstream consumers by removing exports and using incorrect React Context syntax.
React Context Fixes
Task.tsx & EvoluContext.tsx:
testCreateRun()tonull(prevents resource allocation at module load)<Context value={...}>→<Context.Provider value={...}>createEvoluContextcreate unique contexts per call instead of reusing singletonRestored Missing Exports
packages/react/src/index.ts:
createUseEvoluandEvoluProvider(used by multitenant example, react-vite-pwa)packages/vue/src/index.ts:
useEvoluErrorand restored implementationpackages/react-native/src/exports/expo-op-sqlite.ts:
Examples
startscript in react-expo (was renamed to_start)Cleanup
OwnerId,createOwnerWebSocketTransportfromEvolu.tsMaintains backward compatibility: both old API (
createUseEvolu/EvoluProvider) and new API (createEvoluContext) work correctly.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit
Release Notes
New Features
Chores