-
Notifications
You must be signed in to change notification settings - Fork 37
Adding svelte helper #1172
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.
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
base: main
Are you sure you want to change the base?
Adding svelte helper #1172
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
# packages/core/src/actor/instance.ts # packages/frameworks/svelte/package.json # packages/frameworks/svelte/svelte.config.js # packages/frameworks/svelte/tsconfig.json # packages/frameworks/svelte/vite.config.ts # pnpm-lock.yaml
handler: (...args: any[]) => void, | ||
) { | ||
// Get the current connection reactively | ||
const connection = $derived(state.state.connection); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a state access issue in the useEvent
function. The code is trying to access state.state.connection
, but based on the implementation pattern seen elsewhere in the file, this should be state.currentVal.connection
. This incorrect property path will likely cause runtime errors when attempting to access the connection object.
// Current problematic code
const connection = $derived(state.state.connection);
// Should likely be
const connection = $derived(state.currentVal.connection);
const connection = $derived(state.state.connection); | |
const connection = $derived(state.currentVal.connection); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
- `createState()`: Function that returns initial actor state | ||
- `onStart(c)`: Called any time actor is started (after restart/upgrade) | ||
- `onStateChange(c, newState)`: Called when actor state changes | ||
- `onStateChange(c,prevState, newState)`: Called when actor state changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onStateChange
function signature has been updated to include prevState
, but there's a spacing inconsistency in the documentation. For consistency with other function signatures in this file, a space should be added after the first comma:
- `onStateChange(c,prevState, newState)`: Called when actor state changes
+ `onStateChange(c, prevState, newState)`: Called when actor state changes
- `onStateChange(c,prevState, newState)`: Called when actor state changes | |
- `onStateChange(c, prevState, newState)`: Called when actor state changes |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
@joshua1 is attempting to deploy a commit to the Rivet Team on Vercel. A member of the Team first needs to authorize it. |
svelte helper just like react with example project added to the rivetkit core