AB#109285 convert to function based app - #94
Conversation
Deploying canvas-subaccounts with
|
| Latest commit: |
a8c9e2f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28df7b81.canvas-subaccounts.pages.dev |
| Branch Preview URL: | https://ab-109285.canvas-subaccounts.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the React codebase by converting several class-based components to function components using Hooks (useState, useEffect, useCallback, useRef) and memoization (React.memo), aligning the app with a function-based component architecture.
Changes:
- Converted
App,AccountsTree,ListAccounts,Account, andErrorfrom class components to function components. - Reworked
AccountsTreedata-loading and search flow to use Hook state/refs and an iterative search approach. - Updated
package-lock.jsonwith dependency metadata changes (removal oflibcentries for some optional packages).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/App.jsx | Converted to Hooks; token handling and derived server config now computed in-function. |
| src/AccountsTree.jsx | Converted to Hooks; rewritten loading/search/open-state logic with refs and functional updates. |
| src/ListAccounts.jsx | Converted to React.memo function component; internal render helpers moved to closures. |
| src/Account.jsx | Converted to function component; click handler now uses useCallback. |
| src/Error.jsx | Converted to a simple function component. |
| package-lock.json | Removes libc metadata from certain optional packages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| updateState({ open: { ...state.open, ...toOpen }, from: match }, () => accountRefs.current[match]?.scrollIntoView({ behavior: 'smooth', block: 'center' })) | ||
| } else updateState({ searchMessages: [{ type: 'error', text: state.from ? 'No more matches' : 'No matches' }], from: state.from ? null : state.from }) |
| const servers = settings[window.location.origin] | ||
|
|
||
| updateToken = (token) => { | ||
| const updateToken = useCallback((token) => { | ||
| const jwt = jwtDecode(token) | ||
| this.setState({ | ||
| setState(current => ({ ...current, |
buckett
left a comment
There was a problem hiding this comment.
Generally looks ok, although it's hard to reason about.
| const [state, setState] = useState({ search: '', open: {}, tryLoading: true, loadAll: false, loadingAll: false, collections: null }) | ||
| const collectionsRef = useRef(null) | ||
| const accountRefs = useRef([]) | ||
| const updateState = (value, callback) => setState(current => { const next = typeof value === 'function' ? value(current) : value; if (next.collections) collectionsRef.current = next.collections; if (callback) setTimeout(callback, 0); return { ...current, ...next } }) |
There was a problem hiding this comment.
Really hard to read all on one line.
| * We return null to indicate we haven't found anything. | ||
| * An empty array to indicate that the location we were searching from previously has been found | ||
| * A non empty array when we matched | ||
| */ |
| const ordered = [] | ||
| const visit = (id) => { | ||
| ordered.push(id) | ||
| ;(collections[id].collections || []).forEach(visit) |
There was a problem hiding this comment.
What is going on with the formatting here?
af27bf4 to
a8c9e2f
Compare
No description provided.