[V3] Add generic to runtime event handlers#4042
Conversation
|
Warning Rate limit exceeded@IanVS has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces significant improvements to the event handling system in the Wails runtime. The primary focus is on enhancing type safety and flexibility by adding generic type parameters to event-related functions and classes. The changes span multiple files, including the runtime implementation, type definitions, and various frontend template implementations across different frameworks. The modifications aim to provide more precise type checking and clearer event callback definitions while maintaining the existing functionality. Changes
Sequence DiagramsequenceDiagram
participant Client
participant EventSystem
participant Callback
Client->>EventSystem: Register Event Listener
Note over Client,EventSystem: Uses generic type WailsEventCallback<D>
EventSystem-->>Client: Return Unregister Function
Client->>EventSystem: Trigger Event
EventSystem->>Callback: Invoke with Typed Event
Callback->>Client: Process Event Data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
v3/internal/templates/vanilla-ts/frontend/src/main.ts (1)
21-23: Consider renaming parameter for consistency.While the generic type addition improves type safety, consider renaming the parameter from
timetotimeValueto maintain consistency with other templates and avoid confusion with the variable name.-Events.On<string>('time', (time) => { - timeElement.innerText = time.data; +Events.On<string>('time', (timeValue) => { + timeElement.innerText = timeValue.data;v3/internal/runtime/desktop/@wailsio/runtime/src/events.js (1)
Line range hint
45-56: Consider adding type constraints for the generic parameter.While the generic implementation looks good, consider adding type constraints to prevent potential misuse with incompatible types.
-/** - * @template [D=unknown] - */ +/** + * @template [D=unknown] + * @template {object|string|number|boolean|null} D + */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
v3/internal/runtime/desktop/@wailsio/runtime/src/events.js(4 hunks)v3/internal/runtime/desktop/@wailsio/runtime/types/events.d.ts(2 hunks)v3/internal/templates/base/frontend/main.js(1 hunks)v3/internal/templates/lit-ts/frontend/src/my-element.ts(1 hunks)v3/internal/templates/preact-ts/frontend/src/app.tsx(1 hunks)v3/internal/templates/qwik-ts/frontend/src/app.tsx(1 hunks)v3/internal/templates/react-swc-ts/frontend/src/App.tsx(1 hunks)v3/internal/templates/react-ts/frontend/src/App.tsx(1 hunks)v3/internal/templates/solid-ts/frontend/src/App.tsx(1 hunks)v3/internal/templates/svelte-ts/frontend/src/App.svelte(1 hunks)v3/internal/templates/vanilla-ts/frontend/src/main.ts(1 hunks)v3/internal/templates/vanilla/frontend/main.js(1 hunks)v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- v3/internal/templates/base/frontend/main.js
- v3/internal/templates/vanilla/frontend/main.js
🔇 Additional comments (13)
v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue (1)
25-27: LGTM! Clean implementation with proper type safety.The implementation correctly uses the generic type parameter while maintaining Vue's best practices with lifecycle hooks and reactive refs.
v3/internal/templates/solid-ts/frontend/src/App.tsx (1)
23-25: LGTM! Well-implemented with proper type safety.The implementation correctly uses the generic type parameter while following Solid.js best practices with lifecycle hooks and signals.
v3/internal/runtime/desktop/@wailsio/runtime/src/events.js (2)
13-19: LGTM! Well-structured type definition for event callbacks.The new
WailsEventCallbacktype definition with generic parameterDprovides proper type safety for event handlers.
Line range hint
80-110: LGTM! Comprehensive documentation and type safety for event registration methods.The event registration methods (
OnMultiple,On,Once) are well documented with proper generic type parameters and return types.v3/internal/templates/react-ts/frontend/src/App.tsx (1)
23-23: LGTM! Proper usage of generic type parameter.The
Events.On<string>correctly specifies the event data type, allowing TypeScript to infer thetimeValueparameter type.v3/internal/templates/react-swc-ts/frontend/src/App.tsx (1)
23-23: Implementation matches React template.v3/internal/templates/preact-ts/frontend/src/app.tsx (1)
23-23: Implementation matches React template.v3/internal/runtime/desktop/@wailsio/runtime/types/events.d.ts (4)
5-12: LGTM! Well-structured type definition for OnMultiple.The generic type parameter and precise return type enhance type safety while maintaining flexibility with the default
unknowntype.
16-20: LGTM! Consistent type definition for On.The changes maintain consistency with OnMultiple, providing the same level of type safety and flexibility.
24-29: LGTM! Consistent type definition for Once.The changes maintain consistency with other event handlers, completing the set of typed event registration methods.
238-250: LGTM! Well-structured WailsEvent class and callback type.The generic type parameter is properly implemented in both the class and callback type, providing a solid foundation for type-safe event handling.
v3/internal/templates/qwik-ts/frontend/src/app.tsx (1)
23-24: LGTM! Proper usage of generic type parameter.The implementation correctly uses the new
Events.On<string>syntax and properly accesses the event data.v3/internal/templates/lit-ts/frontend/src/my-element.ts (1)
26-28: LGTM! Proper usage of generic type parameter.The implementation correctly uses the new
Events.On<string>syntax and properly accesses the event data.
| Events.On<string>('time', (timeValue) => { | ||
| time = timeValue.data; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider wrapping event listener in onMount.
While the generic type addition is good, the event listener should be wrapped in Svelte's onMount lifecycle function to prevent potential memory leaks and ensure proper cleanup.
+import { onMount } from 'svelte';
-Events.On<string>('time', (timeValue) => {
- time = timeValue.data;
-});
+onMount(() => {
+ Events.On<string>('time', (timeValue) => {
+ time = timeValue.data;
+ });
+});Committable suggestion skipped: line range outside the PR's diff.
|
Hi @leaanthony is there anything else I need to do here to help get this merged? Thanks! |
|
I missed that Edit: this is being pulled into other work that's going on currently, so I'm going to leave it as it is. |
…#5400) examples: run go mod tidy on Windows to fix missing webview2 go.sum entries Six examples (dev, file-association, gin-routing, gin-service, notifications, server) had go.sum files missing the Windows-only webview2 dependency (github.com/wailsapp/wails/webview2), causing `go build .` to fail on Windows with "missing go.sum entry for module providing package github.com/wailsapp/wails/webview2/pkg/edge". Fixes #4042 Co-authored-by: Lea Anthony <lea.anthony@gmail.com>
2 participants
Description
This strengthens the TypeScript types of
@wailsio/runtimeby adding a generic parameter toEvents.On,Events.OnMultipleandEvents.Once, and updates the templates to use the new format.This was discussed in discord: https://discord.com/channels/1042734330029547630/1065600636294549514/1334267494491357204
Fixes # NA
Type of change
Please select the option that is relevant.
How Has This Been Tested?
I copied the newly-generated
events.d.tsinto my own project, and verified that I was able to use the new generic without any type errors:Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Type Safety Improvements
Event Handling Updates
Framework Template Updates