Conversation
Deploying chat with
|
| Latest commit: |
bab9a97
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dd295224.chat-e3y.pages.dev |
| Branch Preview URL: | https://svelte-talon-auth.chat-e3y.pages.dev |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the authentication system from @feathersdev/auth to talon-auth, a web component-based authentication solution. The change consolidates authentication code into the main App component and removes separate module files.
- Replaced
@feathersdev/authwithtalon-authweb component - Removed
auth.tsandautomerge.tsmodule files - Inlined authentication and automerge initialization into
App.svelte
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| svelte-chat/src/automerge.ts | Deleted file - automerge client initialization moved to App.svelte |
| svelte-chat/src/auth.ts | Deleted file - authentication logic replaced with talon-auth web component |
| svelte-chat/src/App.svelte | Integrated talon-auth web component, inlined automerge setup, added onMount lifecycle |
| svelte-chat/package.json | Added talon-auth ^0.12.0 as dev dependency |
| package-lock.json | Added talon-auth package and its dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let text: string = ''; | ||
| let handle: AppDocumentHandle; | ||
|
|
||
| const getAuth = () => document.querySelector('talon-login') as TalonLoginComponent |
There was a problem hiding this comment.
The getAuth() function may return null if the <talon-login> element is not yet rendered in the DOM. This can cause runtime errors when calling methods like getUser() or logoutAndForget(). Consider adding null checking or ensuring the component is mounted before accessing it.
| const getAuth = () => document.querySelector('talon-login') as TalonLoginComponent | |
| const getAuth = () => { | |
| const el = document.querySelector('talon-login') as TalonLoginComponent | null; | |
| if (!el) { | |
| throw new Error('<talon-login> element not found in the DOM'); | |
| } | |
| return el; | |
| } |
|
|
||
| handle = await loadAppDocument(); | ||
| cloudAuthUser = await auth.getUser(); | ||
| const automerge = createAutomerge(getAuth()) |
There was a problem hiding this comment.
The automerge client is created inside the init() function but is not stored in a variable accessible outside this scope. If automerge needs to be referenced elsewhere in the component lifecycle, it should be declared at the component level similar to handle.
| }); | ||
|
|
||
| init(); | ||
| onMount(() => init()) |
There was a problem hiding this comment.
The init() function is async but its Promise is not being handled. Consider either awaiting it or handling potential errors: onMount(() => { init().catch(console.error) })
| onMount(() => init()) | |
| onMount(() => { init().catch(console.error); }) |
1 participant
No description provided.