Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(examples): Fix React projects not providing correct client to app #1191

Closed
wants to merge 9 commits into from
Closed

fix(examples): Fix React projects not providing correct client to app #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
baf46ae
Fix web-wa-sqlite wrong client issue
msfstef Apr 24, 2024
847dce5
Fix tauri strict mode issue
msfstef Apr 24, 2024
22c301b
Fix react native client cleanup
msfstef Apr 24, 2024
c7aecb6
Fix expo client cleanup
msfstef Apr 24, 2024
4d42640
Fix client cleanup on checkout example
msfstef Apr 24, 2024
d594ca3
Fix client cleanup on introduction initialization
msfstef Apr 24, 2024
2619791
Fix client cleanup on recipes
msfstef Apr 24, 2024
436e211
Update react integration docs
msfstef Apr 24, 2024
95d6bc4
Add changeset for template generation
msfstef Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-plants-call.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"create-electric-app": patch
---

Update templates to fix incorrect client providers in React projects
14 changes: 5 additions & 9 deletions docs/integrations/frontend/react.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,21 @@ export const ElectricWrapper = ({ children }) => {
const [ electric, setElectric ] = useState<Electric>()

useEffect(() => {
let isMounted = true
let client: Electric

const init = async () => {
const conn = await ElectricDatabase.init('electric.db')
const electric = await electrify(conn, schema)
client = await electrify(conn, schema)
const token = insecureAuthToken({sub: 'dummy'})
await electric.connect(token)
await client.connect(token)

if (!isMounted) {
return
}

setElectric(electric)
setElectric(client)
}

init()

return () => {
isMounted = false
client?.close()
}
}, [])

Expand Down
20 changes: 8 additions & 12 deletions examples/checkout/src/MainRoutes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,28 @@ interface MainRoutesProps {
onElectricLoaded: () => void
}

const MainRoutes = ({onElectricLoaded}: MainRoutesProps) => {
const { tabId } = uniqueTabId()
const scopedDbName = `basic-${LIB_VERSION}-${tabId}.db`

const MainRoutes = ({ onElectricLoaded }: MainRoutesProps) => {
const [electric, setElectric] = useState<Electric>()
const { supabase } = useContext(SupabaseContext)!

useEffect(() => {
let isMounted = true
let client: Electric

const init = async () => {
const token = await getSupabaseJWT(supabase)

const config = {
debug: true,//import.meta.env.DEV,
debug: true, //import.meta.env.DEV,
url: import.meta.env.ELECTRIC_URL,
}

const { tabId } = uniqueTabId()
const scopedDbName = `basic-${LIB_VERSION}-${tabId}.db`

console.log(config)

const conn = await ElectricDatabase.init(scopedDbName)
const client = await electrify(conn, schema, config)
client = await electrify(conn, schema, config)
await client.connect(token)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The component could have re-rendered during any of the following three awaits.
The returned cleanup function should set an ignore var that is checked after each await, returning immediately if true. This would prevent any unnecessary work and setting the electric state var to the wrong instance.

This is sort of what the isMounted did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue is that electrification affects global components like the registry - the isMounted that we had before (or any check of this kind that we add) will still likely cause a mixup.

Basically the first database connection opened is fed to electrify and starts a satellite process - even if the electrified instance is not set at that point with setElectric, subsequent calls will create new database connections that will be fed to the client but not the satellite as the satellite is global and will use the first one (and first notifier), see:

electric/clients/typescript/src/satellite/registry.ts

Lines 79 to 91 in e92a7e3

// If we're in the process of starting the satellite process for this
// dbName, then we short circuit and return that process. Note that
// this assumes that the previous call to start the process for this
// dbName would have passed in functionally equivalent `dbAdapter`,
// `fs` and `notifier` arguments. Which is *probably* a safe assumption
// in the case where this might happen, which is multiple components
// in the same app opening a connection to the same db at the same time.
const startingPromises = this.startingPromises
const starting = startingPromises[dbName]
if (starting !== undefined) {
return starting
}

This leads to a weird state where the client performs the writes and reads correctly but the satellite is still using the first notifier and adapter/connection.

I find it better to let it do the little additional work (which is only really done in React's StrictMode) and ensure that we end up with the correct setup.

An alternative route, which I think might also be a good solution, is to have a separate, outside of the component top-level initElectric which keeps track of whether it has been initialized or not, and the component doesn't need to worry about multiple calls and cleanups as much.


// This is a simplification for now until we have "shapes"
Expand All @@ -81,18 +81,14 @@ const MainRoutes = ({onElectricLoaded}: MainRoutesProps) => {
await syncBaskets.synced
await syncOrders.synced

if (!isMounted) {
return
}

onElectricLoaded()
setElectric(client)
}

init()

return () => {
isMounted = false
client?.close()
}
}, [supabase])

Expand Down
11 changes: 3 additions & 8 deletions examples/expo/src/ElectricProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,24 @@ const ElectricProviderComponent = ({
const [electric, setElectric] = useState<Electric>()

useEffect(() => {
let isMounted = true

let client: Electric
const init = async () => {
const config = {
debug: DEBUG_MODE,
url: ELECTRIC_URL,
}

const conn = SQLite.openDatabaseSync('electric.db')
const client = await electrify(conn, schema, config)
client = await electrify(conn, schema, config)
await client.connect(authToken())

if (!isMounted) {
return
}

setElectric(client)
}

init()

return () => {
isMounted = false
client?.close()
}
}, [])

Expand Down
12 changes: 4 additions & 8 deletions examples/introduction/src/components/ElectricProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,18 @@ const ElectricProvider = ({ children, dbName }: Props) => {
const [electric, setElectric] = useState<Electric>()

useEffect(() => {
let isMounted = true
let client: Electric

const init = async () => {
const electric = await initElectric(dbName)
client = await initElectric(dbName)

if (!isMounted) {
return
}

setElectric(electric)
setElectric(client)
}

init()

return () => {
isMounted = false
client?.close()
}
}, [])

Expand Down
10 changes: 3 additions & 7 deletions examples/react-native/src/ElectricProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const ElectricProviderComponent = ({children}: {children: React.ReactNode}) => {
const [electric, setElectric] = useState<Electric>();

useEffect(() => {
let isMounted = true;
let client: Electric;

const init = async () => {
const config = {
Expand All @@ -24,20 +24,16 @@ const ElectricProviderComponent = ({children}: {children: React.ReactNode}) => {

const dbName = 'electric.db';
const conn = openSQLiteConnection({name: dbName});
const client = await electrify(conn, dbName, schema, config);
client = await electrify(conn, dbName, schema, config);
await client.connect(authToken());

if (!isMounted) {
return;
}

setElectric(client);
};

init();

return () => {
isMounted = false;
client?.close();
};
}, []);

Expand Down
20 changes: 8 additions & 12 deletions examples/recipes/src/electric/ElectricWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,32 @@ const { ElectricProvider, useElectric } = makeElectricContext<Electric>()
// eslint-disable-next-line react-refresh/only-export-components
export { useElectric }

const { tabId } = uniqueTabId()
const scopedDbName = `recipes-${LIB_VERSION}-${tabId}.db`

export function ElectricWrapper(props: { children: ReactElement[] | ReactElement }) {
const [electric, setElectric] = useState<Electric>()

useEffect(() => {
let isMounted = true
let client: Electric

const init = async () => {
const config = {
debug: import.meta.env.DEV,
url: import.meta.env.ELECTRIC_SERVICE,
}

const { tabId } = uniqueTabId()
const scopedDbName = `recipes-${LIB_VERSION}-${tabId}.db`

const conn = await ElectricDatabase.init(scopedDbName)
const electric = await electrify(conn, schema, config)
await electric.connect(authToken())

if (!isMounted) {
return
}
client = await electrify(conn, schema, config)
await client.connect(authToken())

setElectric(electric)
setElectric(client)
}

init()

return () => {
isMounted = false
client?.close()
}
}, [])

Expand Down