Code style #8
Code style #8
Conversation
| await getProfile(); | ||
| }; | ||
|
|
||
| const getProfile = useCallback(async () => { |
There was a problem hiding this comment.
Read the docs on useCallback but still not clear, can you elaborate on what this in combination with the useEffect dependency achieves?
There was a problem hiding this comment.
so useEffect handler will potentially not trigger on relevant state changes (which can sometimes look like the component is operating on stale data) if you don't pass a dependency array listing all outside variables being used inside the useEffect. So if previously a useEffect did not have the getProfile set, then either the variables the getProfile reads could be stale or there could excessive re-renders and they may not be stale (I think it depends on the component, but it could be one or the other, need to test more). So you need to pass in [getProfile] to useEffect if you want to use it without excessive re-renders and with updated data.
The problem then is that getProfile will be updated on every render and trigger the useEffect unless you wrap it in useCallback, so we need to wrap the function in useCallback and attach a dependency array that ONLY updates the getProfile function when a listed dependency changes @m0ar
I think in this particular case with getProfile only triggering on useEffect on startup, the change may be almost nothing. In the previous case if composeClient or ceramic.did update we may have no re-render, in the new case I believe a re-render would trigger. We may be able to revert this if getProfile is working as expected
There was a problem hiding this comment.
examples on useEffect: https://react.dev/reference/react/useEffect#examples-dependencies
examples on useCallback: https://react.dev/reference/react/useCallback
docs: https://react.dev/reference/react/useEffect#parameters
There was a problem hiding this comment.
Excellent explanation, thanks @hubsmoke 🧁
This is probably the reason behind several things that are a bit odd in the app now, good catch :)
composedb.config.json
Outdated
| "models": [] | ||
| } | ||
| } | ||
| {"anchor":{},"http-api":{"cors-allowed-origins":[".*"],"admin-dids":["did:key:z6Mkewm16NferyW66tYVrSvfUeJHFxtVzbRjjrMgEKxYq5qw"]},"ipfs":{"mode":"bundled"},"logger":{"log-level":2,"log-to-files":true,"log-directory":"local-data/ceramic/logs"},"metrics":{"metrics-exporter-enabled":false,"metrics-port":9090},"network":{"name":"inmemory"},"node":{},"state-store":{"mode":"fs","local-directory":"local-data/ceramic/statestore"},"indexing":{"db":"sqlite:///Users/sinaiman/Dev/desci-composedb/local-data/ceramic/indexing.sqlite","allow-queries-before-historical-sync":true,"models":[]}} |
There was a problem hiding this comment.
yeah, like you pointed out in discord this shouldn't be checked in due to user-specific paths. Mind removing it with git rm?
There was a problem hiding this comment.
FYI: the reason for the absolute paths is that the indexing DB cannot handle relative ones
There was a problem hiding this comment.
I'll add a commit with this removed 👌
| import { ComposeClient } from "@composedb/client"; | ||
|
|
||
| import { definition } from "../src/__generated__/definition.js"; | ||
| import { definition } from "@/src/__generated__/definition.js"; |
There was a problem hiding this comment.
Didn't know about this syntax, nice 💯
There was a problem hiding this comment.
you can set it inside tsconfig.json
| const handleLogin = useCallback(async () => { | ||
| await authenticateCeramic(ceramic, composeClient); | ||
| await setViewerId(); | ||
| }, [ceramic, composeClient, setViewerId]); |
There was a problem hiding this comment.
Here as well, mind elaborating?
| "paths": { | ||
| "@/*": ["./*"] | ||
| } |
|
@hubsmoke another thought that popped up when I saw the vscode settings, in the future I wanna look into Husky for setting up commit hooks for linting/static analysis. If we aim to have OSS contributors we probably shouldn't rely on code style through the configuration of a single editor :) |
…e husky to standardize formatting without vscode
changes to code organization for frontend components. use 'use client' directive to ensure non-server components aren't rendered on server
contains all changes in #7
this is separate from #7 in case you disagree with the code styling but still want the fixes in 7