fix: added EventsOffAll to windows.runtime and updated the frontend runtime wrapper #4254
Conversation
|
""" WalkthroughThe changes introduce a new Changes
Assessment against linked issues
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/src/pages/changelog.mdx (1)
19-20: Fill in the placeholder PR link before merging
???should be replaced with the actual pull-request number (4254).
Leaving placeholders in the changelog reduces traceability for users who rely on the document to navigate to the relevant discussions.- - Added EventsOffAll to `windows.runtime`, fixes [#3136](https://github.com/wailsapp/wails/issues/3136) [PR](https://github.com/wailsapp/wails/pull/???) by [@FrancescoLuzzi](https://github.com/FrancescoLuzzi) + - Added EventsOffAll to `windows.runtime`, fixes [#3136](https://github.com/wailsapp/wails/issues/3136) [PR](https://github.com/wailsapp/wails/pull/4254) by [@FrancescoLuzzi](https://github.com/FrancescoLuzzi)v2/internal/frontend/runtime/wrapper/runtime.js (1)
59-62: Minor slice-usage quirk can be simplified
let args = [eventName].slice.call(arguments);works, but theeventNamearray element is discarded immediately. UsingArray.prototype.sliceis clearer and avoids the superfluous allocation.- let args = [eventName].slice.call(arguments); + let args = Array.prototype.slice.call(arguments);v2/internal/frontend/runtime/desktop/main.js (1)
91-99: Replacedeletewith undefined-assignment to avoid hidden perf costsStatic-analysis flagged two
deleteoperations. In V8 (Chromium/WebView2)deletetriggers dictionary-mode transitions and de-opts hidden classes. Setting the property toundefinedpreserves performance while achieving the same result.- delete window.wails.SetBindings; + window.wails.SetBindings = undefined; ... - delete window.wailsbindings; + window.wailsbindings = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 93-93: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 98-98: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v2/internal/frontend/runtime/desktop/main.js(1 hunks)v2/internal/frontend/runtime/runtime_prod_desktop.js(1 hunks)v2/internal/frontend/runtime/wrapper/runtime.js(1 hunks)website/src/pages/changelog.mdx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
v2/internal/frontend/runtime/desktop/main.js
[error] 93-93: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 98-98: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
v2/internal/frontend/runtime/runtime_prod_desktop.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
v2/internal/frontend/runtime/wrapper/runtime.js (1)
51-53: API surface extended correctly – remember to update typings
EventsOffAllis now exposed through the wrapper – great!
Please ensure the corresponding TypeScript declaration file (runtime.d.ts) is updated, otherwise TS consumers will not “see” the new function.No functional issues spotted.
v2/internal/frontend/runtime/desktop/main.js (1)
55-61:EventsOffAllcorrectly wired into the runtime objectImport + export chain looks consistent with the other event helpers. Nice work.
v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
1-1: Minified build artefact – no source changes to reviewThis file is the generated, minified production bundle. Manual edits will be overwritten by the build pipeline, so no review feedback is required here.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
FrancescoLuzzi
force-pushed
the
bugfix/3136_export_EventsOffAll
branch
from
49de19c to
c576cb4
Compare
FrancescoLuzzi
force-pushed
the
bugfix/3136_export_EventsOffAll
branch
from
c576cb4 to
1de61fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
v2/internal/frontend/runtime/desktop/main.js (3)
16-18: Import ofEventsOffAlllooks correct – minor ordering nitThe new symbol is imported right after
EventsOff, which is semantically fine.
If you keep the list roughly alphabetical (or grouped by “Off/OffAll/On/Once”) it becomes easier to scan when adding/removing future helpers.
55-61: Clarify the distinction betweenEventsOffandEventsOffAll
EventsOffAllis now exposed alongsideEventsOff. Note thatEventsOff()without arguments previously acted as a “remove-all” helper in several client snippets. Introducing a dedicatedEventsOffAllmay create two different public ways of clearing listeners or, worse, break older code that relied on the empty-arg behaviour ofEventsOff.
- Verify how
EventsOffbehaves when called with no arguments inevents.js.- If both APIs coexist, consider:
• Deprecating the empty-arg variant ofEventsOffand documentingEventsOffAllas the canonical helper, or
• Keeping only one public method and aliasing the other for backward compatibility (EventsOffAll = EventsOff).This avoids confusion for downstream users and reduces surface area.
91-99: Avoid thedeleteoperator for hot-path globalsBiome flags both
deletecalls as a performance hit. Replacing them with anundefinedassignment prevents the hidden-class de-opt in V8 while achieving the same GC behaviour.- window.wails.SetBindings(window.wailsbindings); - delete window.wails.SetBindings; + window.wails.SetBindings(window.wailsbindings); + window.wails.SetBindings = undefined; ... - delete window.wailsbindings; + window.wailsbindings = undefined;Because this runs at startup it’s not critical, yet it’s a free micro-optimisation and silences the linter.
🧰 Tools
🪛 Biome (1.9.4)
[error] 93-93: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 98-98: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
v2/internal/frontend/runtime/runtime_prod_desktop.js (2)
1-1:EventsOffAllcorrectly wired, but confirm listener-addition race
EventsOffAll(z) iterates overObject.keys(w)once and then callsp()to unregister each event.
If new listeners are registered during the loop they will survive becauseObject.keyswas taken before the loop.
That’s fine for most use-cases, but if callers expect a strict “no listener survives” guarantee, you may want to:function z() { // keep draining until empty – protects against races let names; while ((names = Object.keys(w)).length) { for (const n of names) p(n); } }Only worth changing if your runtime allows listener registration from inside
EventsOff/OffAllcallbacks.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
1-1: Samedeletepattern as inmain.jsFor consistency and micro-perf reasons, consider the
undefinedassignment approach here as well (e.g. the twodeletestatements right afterSetBindings).🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v2/internal/frontend/runtime/desktop/main.js(1 hunks)v2/internal/frontend/runtime/runtime_prod_desktop.js(1 hunks)v2/internal/frontend/runtime/wrapper/runtime.js(1 hunks)website/src/pages/changelog.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website/src/pages/changelog.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- v2/internal/frontend/runtime/wrapper/runtime.js
🧰 Additional context used
🪛 Biome (1.9.4)
v2/internal/frontend/runtime/desktop/main.js
[error] 93-93: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 98-98: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
v2/internal/frontend/runtime/runtime_prod_desktop.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
1-1: Consider using optional chaining for safer property access.The static analysis identified several instances where optional chaining could be used to make property access safer, particularly when checking for existence of properties before accessing nested ones.
For example, in places where you check for window.chrome?.webview?.postMessageWithAdditionalObjects, using the optional chaining operator consistently would make the code more robust.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v2/internal/frontend/runtime/desktop/main.js(6 hunks)v2/internal/frontend/runtime/runtime_prod_desktop.js(1 hunks)v2/internal/frontend/runtime/wrapper/runtime.js(1 hunks)website/src/pages/changelog.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- website/src/pages/changelog.mdx
- v2/internal/frontend/runtime/wrapper/runtime.js
- v2/internal/frontend/runtime/desktop/main.js
🧰 Additional context used
🪛 Biome (1.9.4)
v2/internal/frontend/runtime/runtime_prod_desktop.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (4)
v2/internal/frontend/runtime/runtime_prod_desktop.js (4)
50-52: The newEventsOffAllfunction is correctly implemented.This new function properly iterates through all event keys in the event store and calls the event removal function on each key, effectively removing all registered event listeners at once.
281-281: Correctly exported the newEventsOffAllfunction in the runtime API.The
EventsOffAllfunction (functionz()in the minified code) is properly exposed in thewindow.runtimeobject, making it available to consumers of the runtime API.
1-2: Overall implementation of the EventsOffAll feature is complete and consistent.The PR successfully implements the
EventsOffAllfunction to remove all event listeners at once, as required by issue #3136. The implementation is consistent with the existing event system pattern and correctly integrates with the runtime API.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
280-280:✅ Verification successful
Verify that the changed
Environmentfunction maintains backward compatibility.The implementation of the
Environmentfunction appears to have changed from returning a direct promise to invoking a Wails command. Ensure this change doesn't break existing code that depends on the previous behavior.
🏁 Script executed:
#!/bin/bash # Search for usages of the Environment function in the codebase rg -A 3 "runtime\.Environment\(" --type jsLength of output: 4929
🏁 Script executed:
#!/bin/bash echo "Files in v2/internal/frontend/runtime:" ls v2/internal/frontend/runtime echo -e "\n--- runtime_prod_desktop.js (lines 260-300) ---" sed -n '260,300p' v2/internal/frontend/runtime/runtime_prod_desktop.js echo -e "\n--- runtime_dev_desktop.js (lines 260-300) ---" if [ -f v2/internal/frontend/runtime/runtime_dev_desktop.js ]; then sed -n '260,300p' v2/internal/frontend/runtime/runtime_dev_desktop.js else echo "runtime_dev_desktop.js not found" fiLength of output: 812
🏁 Script executed:
#!/bin/bash # Show all lines containing "Environment" in runtime_prod_desktop.js with context grep -n -C3 "Environment" v2/internal/frontend/runtime/runtime_prod_desktop.js || echo "No occurrences found"Length of output: 11683
Verify Environment Backward Compatibility
TheEnvironmentAPI still returns a Promise, now via the internala(":wails:Environment")call. All existing calls through the wrapper (wrapper/runtime.js: export function Environment() { return window.runtime.Environment() }) remain unchanged, so code depending on.then(...)orawait Environment()will behave identically.No code changes are required. Please ensure any custom typing or documentation expecting a Promise is in place.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
1-1: Consider switching to optional chaining in future refactors.There are multiple instances in the code where optional chaining could be used to improve readability and robustness, according to the static analysis hints.
-w[t]?.slice()||[] +w[t]?.slice()??[] -window.chrome?.webview?.postMessageWithAdditionalObjects!=null +window.chrome?.webview?.postMessageWithAdditionalObjects!=null -window.chrome?.webview?.postMessageWithAdditionalObjects +window.chrome?.webview?.postMessageWithAdditionalObjects🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2/internal/frontend/runtime/desktop/events.js(1 hunks)v2/internal/frontend/runtime/runtime_prod_desktop.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
v2/internal/frontend/runtime/runtime_prod_desktop.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
v2/internal/frontend/runtime/desktop/events.js (2)
193-193: The loop condition is now more conventional.Changing from
i !== eventNames.lengthtoi < eventNames.lengthmakes the iteration pattern more standard and easier to understand at a glance.
191-196: Well-implemented EventsOffAll function.This function properly unregisters all event listeners by iterating through all event names and calling removeListener on each one. The implementation is clean and follows the same pattern used in other event-related functions in this file.
v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
1-1: The EventsOffAll function is successfully exposed in the runtime API.The EventsOffAll function (implemented as
z()) is now correctly exposed in the window.runtime object alongside other event management functions, which satisfies the PR objective.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
FrancescoLuzzi
force-pushed
the
bugfix/3136_export_EventsOffAll
branch
from
d12721b to
21e0bfa
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
v2/internal/frontend/runtime/runtime_prod_desktop.js (3)
353-358: Consider performance impact of using delete operatorThe static analysis hints suggest avoiding the delete operator for performance reasons. In functions like
g(e)that usedelete w[e], consider using an assignment to undefined instead (e.g.,w[e] = undefined).This change is optional but might offer a slight performance improvement, especially in performance-sensitive event handling code.
function g(e){ - delete w[e], + w[e] = undefined, window.WailsInvoke("EX"+e) }
240-242: Optional chaining opportunity for better readabilityMultiple places in the code check for the existence of objects before accessing their properties. Consider using optional chaining for cleaner code, which is supported in modern browsers.
- function V(){return window.chrome?.webview?.postMessageWithAdditionalObjects!=null} + function V(){return window.chrome?.webview?.postMessageWithAdditionalObjects != null} - function Me(e,t,n){window.chrome?.webview?.postMessageWithAdditionalObjects&&chrome.webview.postMessageWithAdditionalObjects(`file:drop:${e}:${t}`,n)} + function Me(e,t,n){window.chrome?.webview?.postMessageWithAdditionalObjects?.(`file:drop:${e}:${t}`,n)}Also applies to: 257-257
1-1: Consider adding JSDoc comments for the new EventsOffAll functionWhile the implementation is correct, consider adding JSDoc comments to document the purpose and behavior of the new
EventsOffAllfunction. This would improve maintainability and help other developers understand its intended use.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2/internal/frontend/runtime/desktop/events.js(1 hunks)v2/internal/frontend/runtime/runtime_prod_desktop.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- v2/internal/frontend/runtime/desktop/events.js
🧰 Additional context used
🪛 Biome (1.9.4)
v2/internal/frontend/runtime/runtime_prod_desktop.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1-1: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
v2/internal/frontend/runtime/runtime_prod_desktop.js (3)
52-52: Addition of EventsOffAll function implements requested functionalityThe new function
z()implements theEventsOffAllfunctionality correctly by iterating through all registered event keys and removing each event listener. This properly addresses issue #3136 mentioned in the PR objectives.
376-376: EventsOffAll is properly exposed in the runtime APIThe function is correctly exported as part of the
window.runtimeobject, making it available for public use alongside other event-related methods.
52-52:✅ Verification successful
Verify integration with other components
The implementation looks correct, but let's ensure it works properly with other components that might rely on the events system.
Also applies to: 376-376
🏁 Script executed:
#!/bin/bash # Check if the EventsOffAll function is used anywhere else in the codebase echo "Searching for any usage of EventsOffAll across the codebase..." rg "EventsOffAll" --type js # Check if there are any tests for this functionality echo "Checking for tests related to EventsOffAll..." rg "EventsOffAll" --type js --glob "*test*"Length of output: 13289
Integration of EventsOffAll Verified
EventsOffAllis referenced in:
v2/internal/frontend/runtime/wrapper/runtime.jsv2/internal/frontend/runtime/desktop/events.jsv2/internal/frontend/runtime/desktop/main.js- Both
runtime_debug_desktop.jsandruntime_prod_desktop.js- Covered by unit tests in
v2/internal/frontend/runtime/desktop/events.test.jsAll integration points are exercised and tested; no further action required.
Description
Add
EventsOffAlltowindow.runtimeand fix the relative frontend bindingsFixes #3136
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
EventsOffAll, to the runtime API, enabling users to remove all event listeners at once.Documentation
EventsOffAllto the runtime API.