[v3] TypeScript runtime#4100
Conversation
WalkthroughThe PR enhances CI/CD workflows by adding a new job to verify runtime build artifacts and standardizing working directory paths. The npm publishing workflow now supports manual triggering, improved artifact cleaning, bundled runtime building, and version bumping. Documentation has been updated with new cancellation features and clearer runtime initialization notes. Several examples now showcase cancellation via both async and chaining approaches, and extensive refactoring has occurred across runtime, generator templates, and TypeScript configuration to integrate cancellable promise support. Changes
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (37)
v3/internal/runtime/desktop/@wailsio/runtime/src/contextmenu.ts (4)
21-21: Type annotations added to function signatureGood addition of type annotations to the function parameters, enhancing type safety. Consider using a more specific type than
anyfor thedataparameter if the structure is known.
37-42: Constants should be used instead of let for variables that aren't reassignedThe variables
customContextMenuanddataare not reassigned, so they should useconstinstead ofletto follow TypeScript best practices.- let customContextMenu = window.getComputedStyle(target).getPropertyValue("--custom-contextmenu").trim(); + const customContextMenu = window.getComputedStyle(target).getPropertyValue("--custom-contextmenu").trim(); if (customContextMenu) { event.preventDefault(); - let data = window.getComputedStyle(target).getPropertyValue("--custom-contextmenu-data"); + const data = window.getComputedStyle(target).getPropertyValue("--custom-contextmenu-data"); openContextMenu(customContextMenu, event.clientX, event.clientY, data);
57-71: Consider extracting duplicate target resolution logicThe logic for resolving the target element is duplicated in both
contextMenuHandlerandprocessDefaultContextMenu. Consider extracting this into a separate function to reduce duplication.+function resolveTarget(event: MouseEvent): HTMLElement { + if (event.target instanceof HTMLElement) { + return event.target; + } else if (!(event.target instanceof HTMLElement) && event.target instanceof Node) { + return event.target.parentElement ?? document.body; + } else { + return document.body; + } +} function contextMenuHandler(event: MouseEvent) { - let target: HTMLElement; - - if (event.target instanceof HTMLElement) { - target = event.target; - } else if (!(event.target instanceof HTMLElement) && event.target instanceof Node) { - target = event.target.parentElement ?? document.body; - } else { - target = document.body; - } + const target = resolveTarget(event); // ... } function processDefaultContextMenu(event: MouseEvent) { // ... - let target: HTMLElement; - - if (event.target instanceof HTMLElement) { - target = event.target; - } else if (!(event.target instanceof HTMLElement) && event.target instanceof Node) { - target = event.target.parentElement ?? document.body; - } else { - target = document.body; - } + const target = resolveTarget(event); // ... }
89-91: Address switch statement declaration warningsThe static analysis tool flagged that variable declarations within switch statements can be accessed by other clauses. Wrap these declarations in a block to restrict their scope and avoid potential issues.
default: // Check if contentEditable is true if (target.isContentEditable) { return; } // Check if text has been selected + { const selection = window.getSelection(); const hasSelection = selection && selection.toString().length > 0; if (hasSelection) { for (let i = 0; i < selection.rangeCount; i++) { // ... } } + }🧰 Tools
🪛 Biome (1.9.4)
[error] 89-89: 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] 90-90: 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)
v3/internal/runtime/desktop/@wailsio/runtime/src/wml.ts (3)
23-25: Prefer a more specific type thananyfor event data.
Usinganyfordatamay reduce type safety and maintainability. If possible, provide a stricter type to enhance clarity and catch potential usage errors.
157-174: Ensure all triggered elements are processed before breaking out.
The loop exits early once[elementCountSym]hits zero, which may leave unvisited elements with triggers if they appear later. Consider removing or adjusting the condition to avoid skipping elements.
203-209: Monitor potential performance impact of the global query selector.
Usingdocument.body.querySelectorAll('*')can be costly for very large pages. If necessary, consider narrowing the search scope to specific containers or employing a more selective query to improve performance.v3/examples/binding/README.md (1)
10-11: Fix markdown list indentationThe static analysis tool flagged inconsistent indentation in the unordered list items. Consider adjusting to use consistent indentation.
- - The bindings generator is still a work in progress and is subject to change. - - The generated code uses `wails.CallByID` by default. This is the most robust way to call a function. +- The bindings generator is still a work in progress and is subject to change. +- The generated code uses `wails.CallByID` by default. This is the most robust way to call a function.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
v3/examples/cancel-async/GreetService.go (1)
8-9: Consider adding a constructor or comments for ServiceThe Service struct is currently empty. If there are plans to add state to this struct in the future, consider adding a constructor function or comments explaining its intended use.
v3/internal/runtime/desktop/@wailsio/runtime/src/flags.ts (1)
1-23: Good implementation with robust error handlingThe TypeScript implementation provides a clean way to access Wails flags with proper error handling. The use of the
causeproperty in the Error constructor is a good modern practice that preserves error chain information.Consider defining TypeScript interfaces for the global
window._wailsobject to make the code more type-safe. This would help catch potential errors at compile time rather than runtime.interface WailsRuntime { flags: Record<string, any>; // other properties... } declare global { interface Window { _wails: WailsRuntime; } }v3/internal/runtime/desktop/@wailsio/runtime/src/promises_aplus.test.js (1)
24-25: Consider using optional chainingThe condition checking can be simplified with optional chaining.
- if (fn && fn.length) { + if (fn?.length) {🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
v3/tasks/events/generate.go (4)
190-190: Potential unescaped string values
When appending raw input (strings.TrimSpace(string(line))) into the TypeScript string, consider escaping quotes or special characters to avoid potential breakage or injection in the generated code if unexpected characters appear in the event names.
204-204: Potential unescaped string values (mac events)
Similarly, for Mac events, ensure thatstrings.TrimSpace(string(line))doesn't introduce quotations or other special characters that could break the generated TypeScript.
252-252: Potential unescaped string values (common events)
The same logic applies to common events. Consider adding an escaping mechanism to safeguard the generated TypeScript if the event file ever contains quotes.
264-264: Potential unescaped string values (Windows events)
The Windows events generation should also ensure that all special characters inlineget properly escaped to avoid code injection or syntax errors.v3/internal/generator/render/create.go (2)
216-223: Conditional TS/JSDoc signature wrapping
Definingpreandpostbased on the presence of type parameters andm.TShelps keep TypeScript vs. JSDoc variants clear. Ensure that any special characters inpp.paramsget validated or escaped if user input is ever passed here.
241-241: Generic argument type of: any[]
Using: any[]is broad and may reduce strict type checks. Evaluate whether narrower types are feasible for improved correctness.v3/internal/runtime/desktop/@wailsio/runtime/src/utils.ts (1)
16-16: Relaxed parameter type
Switchingmessagetoanymakes logging more flexible but trades off compile-time safety. If you require strict type checks, consider using a narrower type.docs/src/content/docs/learn/runtime.mdx (1)
69-73: Grammar issue in caution note.The caution note contains a grammatical error with the comma before "because".
-It is important to include the `type="module"` attribute on the `<script>` tag that loads the runtime -and to wait for the page to be fully loaded before calling the API, -because scripts with the `type="module"` attribute run asynchronously. +It is important to include the `type="module"` attribute on the `<script>` tag that loads the runtime +and to wait for the page to be fully loaded before calling the API +because scripts with the `type="module"` attribute run asynchronously.🧰 Tools
🪛 LanguageTool
[formatting] ~71-~71: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...o be fully loaded before calling the API, because scripts with thetype="module"attrib...(COMMA_BEFORE_BECAUSE)
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/kvstore/keyvaluestore.js (1)
41-41: Function name shadows global propertyThe function name
Setshadows the globalSetconstructor, which could lead to confusion or bugs.Consider renaming this function to avoid shadowing the global
Setobject, perhaps to something likeSetValueorSetKeyValue.-export function Set(key, value) { +export function SetValue(key, value) { return $Call.ByID(2329265830, key, value); }Note: Since this is a generated file, you may need to modify the generator instead.
🧰 Tools
🪛 Biome (1.9.4)
[error] 41-41: Do not shadow the global "Set" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
v3/internal/runtime/desktop/@wailsio/runtime/src/callable.ts (1)
76-76: Sparse array usageUsing a sparse array (
[,]) is generally discouraged as it can lead to unexpected behavior across different JavaScript engines.Replace the sparse array with an explicit undefined value for better compatibility:
-var isIE68 = !(0 in [,]); // eslint-disable-line no-sparse-arrays, comma-spacing +var isIE68 = !(0 in [undefined]); // More explicit and compatible🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.test.js (1)
5-15: Consider implementing and testing the TODO items before merging.These TODOs outline critical edge cases (e.g., cancellation of subpromises, multiple
cancel()calls) that are untested. Completing these tests will improve reliability and reduce the likelihood of bugs.Do you want me to open a new issue to track adding these tests or generate boilerplate tests for these scenarios?
v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts (2)
13-13: Consider making the URL configurable for testing or alternate environments.Hardcoding
window.location.origin + "/wails/runtime"may limit the ability to spin up alternative runtime endpoints (e.g., for local dev vs. staging). A configurable constant could be beneficial.
44-67: Validate error handling for failed calls.Currently, failed fetch calls or unexpected HTTP status codes raise a generic error. For debugging and support, consider including more contextual data (e.g., status code, method name) in the thrown error to provide clarity.
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/log/loggerservice.js (1)
19-20: Reduce duplication by extracting a shared function.All these exported functions (
Debug,Error,Info,SetLogLevel,Warning) follow the same pattern. Consider a helper that takes an ID plus arguments to streamline maintenance.Also applies to: 28-29, 37-38, 45-46, 54-55
v3/internal/runtime/desktop/@wailsio/runtime/src/calls.ts (3)
15-19: Global handlers and state initialization.
Definingwindow._wailsand hooking upcallResultHandler&callErrorHandleris straightforward. Be mindful of potential naming collisions on the globalwindowobject, though it’s typical for Wails runtime usage.
26-28: Constants for call binding.
Using numeric constants is a minimal approach for method routing. Consider adding inline comments or enumerations to clarify the meaning of these constants if additional usage grows.
165-209:Callfunction with cancellable logic.
• Creates a unique ID.
• Attaches promise resolvers in a map.
• Implements run-state checking and acancelroutine viacancelCall.
• Usesoncancelledfor graceful cancellation.
All of this is well-structured. Consider using console.error instead of console.log in the.catchblock if the error is critical.v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.ts (1)
368-368: Consider avoiding the direct override of the 'then' method.
Overriding thethenproperty in a class is flagged by the linter as suspicious. While extendingPromisecan be valid in advanced scenarios, this approach may introduce unexpected behavior or incompatibilities with standard Promise usage.🧰 Tools
🪛 Biome (1.9.4)
[error] 368-368: Do not add then to a class.
(lint/suspicious/noThenProperty)
.github/workflows/publish-npm.yml (1)
27-27: Consider upgrading actions/setup-node to v4.
The linter warns that v3 might be outdated for some runners. A newer version can ensure compatibility.- uses: actions/setup-node@v3 + uses: actions/setup-node@v4🧰 Tools
🪛 actionlint (1.7.4)
27-27: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
v3/internal/assetserver/bundledassets/runtime.js (4)
1-2: Consider modularizing this file.The file is quite large and handles multiple concerns (event management, promise cancellation, window operations, system utilities, etc.). Splitting the code into smaller modules or files would improve readability, maintainability, and make unit testing more straightforward.
🧰 Tools
🪛 Biome (1.9.4)
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
1-2: Evaluate potential performance improvements for repeated event string definitions.There are numerous static strings representing system events for different platforms (Mac, Windows, Linux). Consider generating them programmatically (e.g., code generation or storing definitions in JSON or an object) to reduce manual upkeep and potential duplication.
🧰 Tools
🪛 Biome (1.9.4)
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
1-2: Assess the necessity of the global_wailsproperties.You are adding multiple global variables under
window._wails(e.g.,window._wails.callResultHandler,window._wails.callErrorHandler, etc.). This might conflict with other code in the global space. Consider using a more scoped approach or a dedicated global namespace to reduce the risk of naming collisions.🧰 Tools
🪛 Biome (1.9.4)
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
1-2: CancellablePromise usage looks solid, but consider documenting the cancellation flow.The
CancellablePromiseclass is extensive and well-structured. However, future contributors might find it helpful to have doc comments explaining the cancellation lifecycle, especially since the class includes advanced features (e.g., cancelOn, custom oncancelled callback).🧰 Tools
🪛 Biome (1.9.4)
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
v3/internal/runtime/desktop/@wailsio/runtime/package.json (1)
30-42: Build scripts are comprehensive but can be simplified.You've added multiple separate scripts (
build:code,build:docs,build:docs:md). Consider if these could be consolidated or if the workflow requires them to remain separate. Fewer commands can be easier to maintain.v3/internal/generator/testdata/tsconfig.json (1)
26-27: 'noUnusedParameters': false is acceptable, but be mindful.Allowing unused parameters can sometimes hide logic errors or leftover refs. If there's a strong reason to keep it, that's fine, but consider flipping it on if the code accumulates unused params.
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
14-61: Consider grouping constants in an enum for better maintainability.Currently, these are declared as distinct constants for each method identifier. Converting them into a TypeScript
enumcould make them more organized, cohesive, and less error-prone.Example refactor:
enum WindowMethod { Position = 0, Center, Close, DisableSizeConstraints, EnableSizeConstraints, // ... ZoomReset } const PositionMethod = WindowMethod.Position; // or simply use `WindowMethod.Position` directly below
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
v3/internal/runtime/desktop/@wailsio/runtime/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (82)
.github/workflows/build-and-test-v3.yml(5 hunks).github/workflows/publish-npm.yml(1 hunks)docs/src/content/docs/changelog.mdx(3 hunks)docs/src/content/docs/guides/menus.mdx(1 hunks)docs/src/content/docs/learn/bindings.mdx(2 hunks)docs/src/content/docs/learn/context-menu.mdx(1 hunks)docs/src/content/docs/learn/runtime.mdx(2 hunks)v3/Taskfile.yaml(2 hunks)v3/examples/binding/README.md(1 hunks)v3/examples/binding/assets/bindings/github.com/wailsapp/wails/v3/examples/binding/greetservice.js(2 hunks)v3/examples/cancel-async/GreetService.go(1 hunks)v3/examples/cancel-async/README.md(1 hunks)v3/examples/cancel-async/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-async/index.js(1 hunks)v3/examples/cancel-async/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-async/service.js(1 hunks)v3/examples/cancel-async/main.go(1 hunks)v3/examples/cancel-chaining/GreetService.go(1 hunks)v3/examples/cancel-chaining/README.md(1 hunks)v3/examples/cancel-chaining/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-chaining/index.js(1 hunks)v3/examples/cancel-chaining/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-chaining/service.js(1 hunks)v3/examples/cancel-chaining/main.go(1 hunks)v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/examples/services/hashes/service.js(1 hunks)v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/kvstore/keyvaluestore.js(1 hunks)v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/log/loggerservice.js(2 hunks)v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/sqlite/service.js(1 hunks)v3/examples/services/assets/index.html(2 hunks)v3/internal/assetserver/bundledassets/runtime.js(1 hunks)v3/internal/generator/render/create.go(3 hunks)v3/internal/generator/render/info.go(2 hunks)v3/internal/generator/render/templates/models.js.tmpl(1 hunks)v3/internal/generator/render/templates/models.ts.tmpl(1 hunks)v3/internal/generator/render/templates/service.js.tmpl(3 hunks)v3/internal/generator/render/templates/service.ts.tmpl(4 hunks)v3/internal/generator/testdata/tsconfig.json(1 hunks)v3/internal/runtime/.gitignore(1 hunks)v3/internal/runtime/Taskfile.yaml(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/.npmignore(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/package.json(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/application.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/browser.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/browser.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/callable.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/calls.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/calls.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.test.js(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/clipboard.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/clipboard.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/contextmenu.ts(2 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/create.ts(4 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/drag.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/drag.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts(5 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/events.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/events.test.js(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/events.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/flags.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/flags.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/global.d.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/index.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/index.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/listener.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/nanoid.ts(2 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/promises_aplus.test.js(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/screens.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/screens.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/system.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/system.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/utils.ts(3 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/window.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/wml.js(0 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/wml.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/tsconfig.json(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/vitest.config.ts(1 hunks)v3/internal/runtime/desktop/README.md(1 hunks)v3/internal/runtime/desktop/compiled/main.js(1 hunks)v3/pkg/application/messageprocessor_call.go(1 hunks)v3/tasks/events/generate.go(9 hunks)
💤 Files with no reviewable changes (14)
- v3/internal/runtime/desktop/@wailsio/runtime/src/clipboard.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/browser.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/window.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/index.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/flags.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/wml.js
- v3/internal/runtime/desktop/@wailsio/runtime/.npmignore
- v3/internal/runtime/desktop/@wailsio/runtime/src/drag.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/events.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/screens.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/calls.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.js
- v3/internal/runtime/desktop/@wailsio/runtime/src/system.js
✅ Files skipped from review due to trivial changes (11)
- v3/examples/cancel-chaining/README.md
- v3/examples/cancel-async/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-async/index.js
- v3/internal/runtime/.gitignore
- v3/examples/cancel-async/README.md
- v3/examples/cancel-chaining/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-chaining/index.js
- docs/src/content/docs/guides/menus.mdx
- v3/internal/generator/render/templates/models.js.tmpl
- v3/internal/runtime/desktop/compiled/main.js
- v3/internal/generator/render/templates/models.ts.tmpl
- v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts
- v3/examples/cancel-chaining/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-chaining/service.js
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
v3/examples/binding/README.md
10-10: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🪛 LanguageTool
docs/src/content/docs/learn/runtime.mdx
[formatting] ~71-~71: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...o be fully loaded before calling the API, because scripts with the type="module" attrib...
(COMMA_BEFORE_BECAUSE)
[misspelling] ~79-~79: Did you mean “side effect” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...n't use the API, make sure to include a side-effect import statement somewhere in your fron...
(AFFECT_EFFECT)
[misspelling] ~86-~86: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...r bundler should detect the presence of side-effects and include all required initialisation...
(AFFECT_EFFECT)
docs/src/content/docs/learn/bindings.mdx
[uncategorized] ~440-~440: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...e special cancel method on the promise and it will reject immediately with a speci...
(COMMA_COMPOUND_SENTENCE)
🪛 Biome (1.9.4)
v3/internal/runtime/desktop/@wailsio/runtime/src/promises_aplus.test.js
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
v3/internal/runtime/desktop/@wailsio/runtime/src/callable.ts
[error] 76-76: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/log/loggerservice.js
[error] 27-27: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/kvstore/keyvaluestore.js
[error] 41-41: Do not shadow the global "Set" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
v3/internal/runtime/desktop/@wailsio/runtime/src/contextmenu.ts
[error] 89-89: 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] 90-90: 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)
v3/internal/runtime/desktop/@wailsio/runtime/src/system.ts
[error] 35-37: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.ts
[error] 228-228: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
v3/internal/runtime/desktop/@wailsio/runtime/src/create.ts
[error] 31-31: Do not shadow the global "Array" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 52-52: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.ts
[error] 368-368: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 924-925: Do not add then to an object.
(lint/suspicious/noThenProperty)
v3/internal/assetserver/bundledassets/runtime.js
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
🪛 actionlint (1.7.4)
.github/workflows/publish-npm.yml
27-27: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests (macos-latest, 1.24)
- GitHub Check: Run Go Tests (windows-latest, 1.24)
🔇 Additional comments (175)
docs/src/content/docs/learn/context-menu.mdx (1)
68-71: Clarify the Runtime Initialization NoteThe newly added note clearly informs users that the context menu feature requires the runtime to be initialized before it functions as expected. This is important information since missing this step could lead to confusion. Please ensure that similar documentation (e.g., in
docs/src/content/docs/guides/menus.mdx) remains consistent with this note.v3/internal/runtime/desktop/@wailsio/runtime/src/contextmenu.ts (4)
11-12: Import paths correctly include .js extension for ES modulesThe import statements correctly include the
.jsextension which is required when using ES modules in the browser, even for TypeScript files.
17-18: Function updated to use newRuntimeCallerProperly updated to use the new
newRuntimeCallerfunction instead of the previousnewRuntimeCallerWithIDfunction, which aligns with the TypeScript migration.
25-34: Improved event target handlingGood enhancement to properly handle cases where the event target is not an HTMLElement, ensuring a valid target is always assigned.
104-106: Improved type safety for form element checksGood improvement on type safety by using
instanceofchecks for HTMLInputElement and HTMLTextAreaElement rather than checking tag names directly.v3/internal/runtime/desktop/@wailsio/runtime/src/wml.ts (3)
33-47: Check for undefinedtargetWindowbefore invoking methods.
Window.Get(windowName)could returnundefinedif the window doesn't exist. An extra guard would prevent runtime errors whentargetWindowis invalid.Do you want me to generate a script to locate all usages of
Window.Get()in the codebase to confirm safe usage?
52-70: No major issues found with WML event handling.
The logic for conditional confirmation and subsequent actions appears consistent and robust. The function gracefully handles absent attributes.
199-201: Initialization approach looks good.
CallingwhenReady(Reload)ensures WML listeners are appropriately set after the DOM is fully loaded.v3/internal/runtime/desktop/@wailsio/runtime/vitest.config.ts (2)
1-1: Style consistency improvementsThe style changes (double quotes for imports and added semicolons) are good for consistency, aligning with TypeScript best practices.
Also applies to: 8-8
6-6:Details
❓ Verification inconclusive
Consider reviewing the test timeout value
Setting a 200ms timeout may be too restrictive for some tests, potentially causing intermittent failures if any tests occasionally run longer than expected. Consider increasing this value unless you have specific performance requirements for keeping tests this fast.
🏁 Script executed:
#!/bin/bash # Check if there are any tests that might be at risk with this timeout rg -l "setTimeout|setInterval|delay|sleep" --type js --type ts "v3/internal/runtime/desktop/@wailsio/runtime/src" | grep -v "node_modules"Length of output: 269
Review Update: Reevaluate Test Timeout Value
The
vitest.config.tscurrently sets atestTimeoutof 200ms. However, a grep of the source files shows that tests in the cancellable modules (found inv3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.test.jsandv3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.ts) make use of asynchronous functions (e.g., viasetTimeout, etc.). This suggests that a 200ms test timeout may be too restrictive and could lead to intermittent failures under normal asynchronous operations.
- File:
v3/internal/runtime/desktop/@wailsio/runtime/vitest.config.ts(line 6)- Observation: Presence of asynchronous calls in tests (e.g., in cancellable modules)
- Recommendation: Review if tests can reliably complete within 200ms. If not, consider increasing the timeout to reduce the risk of intermittent failures unless there’s a strict performance requirement.
v3/examples/binding/README.md (2)
5-5: New-cleanflag addition is a good improvementAdding the
-cleanflag to the bindings generation command will ensure a fresh generation of bindings, preventing potential issues from stale generated code.
11-11: "Robust" description aligns better with the PR objectivesChanging from "most secure and quickest" to "most robust" better reflects the emphasis on improved cancellation and error handling introduced in this PR, particularly with the new
CancellablePromiseimplementation.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
v3/examples/cancel-async/GreetService.go (1)
11-19: Well-implemented cancellable operation exampleThis is a clean implementation of a cancellable long-running operation following Go best practices:
- Properly uses context for cancellation
- Clear select statement handling both completion and cancellation cases
- Returns appropriate errors when cancelled
This example aligns perfectly with the PR objective of introducing cancellable promises and improving cancellation behavior.
v3/internal/runtime/desktop/README.md (2)
3-4: Clear documentation of entry point is helpfulExplicitly documenting that
index.jsin thecompileddirectory is the entry point forruntime.jsprovides clear guidance for developers.
8-10: Valuable CI process documentationThe warning about not manually rebuilding the runtime is excellent documentation that will prevent workflow issues. Clear explanation that:
- CI will handle rebuilding
- PRs affecting build artifacts will be blocked
This aligns with the PR objectives by ensuring controlled implementation of the TypeScript migration.
v3/internal/generator/render/info.go (2)
3-8: Import added appropriately.The
regexppackage has been properly imported to support the new regex functionality.
30-31: Clear documentation for new regex pattern.The comment clearly explains the purpose of the new
createParamRegexvariable, which is essential for matching type parameter creation strings generated by themodelinfofunction.v3/examples/cancel-chaining/GreetService.go (2)
1-9: Well-structured service definition.The package structure and imports are correctly defined for a new Go service example.
11-19: Excellent implementation of cancellable long-running operation.The
LongRunningmethod properly implements the context cancellation pattern in Go. It correctly:
- Uses
time.Afterfor the timeout- Listens for context cancellation
- Returns the appropriate error from context when cancelled
This implementation aligns perfectly with the PR objective of enhancing cancellable promises.
v3/internal/runtime/desktop/@wailsio/runtime/src/global.d.ts (3)
1-10: Well-formatted header comment.The header comment provides clear attribution and properly identifies the project.
11-15: Proper TypeScript declaration for Wails global state.The global interface extension for Window is correctly implemented, allowing TypeScript to recognize the
_wailsproperty that stores runtime state.
17-17: Correct module declaration.The empty export ensures this file is treated as a module while still declaring global types, which is the recommended TypeScript pattern.
v3/examples/cancel-async/assets/bindings/github.com/wailsapp/wails/v3/examples/cancel-async/service.js (3)
1-7: Proper TypeScript integration in generated JS file.The file correctly includes TypeScript checks and imports the necessary components from the Wails runtime, including the new
CancellablePromisetype.
9-13: Well-documented function with TypeScript return type.The JSDoc comment properly documents the function purpose and parameters, and includes the correct TypeScript return type annotation using the new
$CancellablePromise<void>type.
14-16: Proper implementation of cancellable binding call.The
LongRunningfunction correctly uses$Call.ByIDwith the appropriate parameters, returning a cancellable promise as described in the PR objectives.v3/internal/runtime/desktop/@wailsio/runtime/src/browser.ts (1)
1-24: Well-implemented TypeScript migration with proper type safetyThis is a clean implementation of the browser functionality in TypeScript. The function correctly accepts both string and URL types as parameters, providing good flexibility. The JSDoc documentation is comprehensive and follows best practices.
v3/examples/cancel-async/main.go (1)
1-37: Good example implementation for the cancellable promises featureThis example application is well-structured and follows Go best practices. It properly demonstrates the setup needed for the cancellable promises feature mentioned in the PR objectives. The code includes proper error handling and clear structure.
v3/Taskfile.yaml (1)
99-127: Properly updated test configuration with new examplesThe task configuration has been appropriately updated to include the new example directories in the test process. This ensures the new features like cancellable promises (cancel-async, cancel-chaining) and other functionality are properly tested.
v3/examples/cancel-chaining/main.go (1)
1-37: Clean implementation of a Wails v3 application setupThis is a well-structured example demonstrating an application that can utilize cancellable promises. The code follows standard Wails v3 patterns for setting up an application with services, assets, and a webview window.
I particularly like the clear organization and the appropriate error handling at the end of the main function.
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/examples/services/hashes/service.js (3)
7-7: Imported CancellablePromise for improved promise handlingThe import statement has been updated to include the
CancellablePromiseclass, which aligns with the PR objective of enhancing cancellation for long-running binding calls.
15-15: Updated return type to use CancellablePromiseThe return type annotation now correctly specifies
$CancellablePromise<$models.Hashes>, which properly reflects the breaking change mentioned in the PR objectives where cancelled binding calls will now reject with aCancelError.
18-18: Simplified implementation with direct promise returnThe implementation has been simplified to directly return the call result without an intermediate variable, making the code more concise while maintaining the same functionality.
v3/internal/runtime/desktop/@wailsio/runtime/src/clipboard.ts (1)
1-35: Well-structured TypeScript implementation of clipboard functionsThis new TypeScript file is a clean implementation of clipboard functionality with appropriate type annotations, JSDoc comments, and a clear separation of concerns. The implementation follows the runtime caller pattern consistently.
The functions are well-documented, making the API easy to understand for developers integrating with the clipboard functionality.
v3/internal/runtime/desktop/@wailsio/runtime/src/nanoid.ts (2)
30-31: Improved variable declaration with constChanged from
lettoconstforurlAlphabetwhich is appropriate since this value is not reassigned. This follows TypeScript best practices for immutable values.
33-42: Enhanced function declaration with proper TypeScript annotationsThe
nanoidfunction has been updated from an arrow function to a standard function declaration with explicit type annotations for the parameter and return value. This improves type safety and makes the API contract clearer.v3/examples/services/assets/index.html (2)
51-51: Simplified success message handlingThe code now uses a simple static success message rather than potentially appending the result data. This change simplifies the success feedback for non-select queries.
77-78: Good addition of error loggingAdding console.error logging improves debugging capabilities by providing more visibility into SQL execution failures in the developer console alongside the user-facing error message.
v3/internal/runtime/desktop/@wailsio/runtime/src/promises_aplus.test.js (2)
1-44: Excellent implementation of Promise/A+ compliance testingThis file properly sets up a test harness to ensure the new CancellablePromise implementation adheres to the Promise/A+ specification, which is critical for ensuring compatibility with existing code.
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
33-44: Well-structured adapter for Promise/A+ testingThe setup correctly configures the testing library to use CancellablePromise for all promise-related operations, ensuring thorough compliance testing.
v3/internal/runtime/desktop/@wailsio/runtime/src/application.ts (2)
11-12: Improved runtime caller implementationThe change from
newRuntimeCallerWithIDtonewRuntimeCallersimplifies the API usage and makes the code more maintainable.
21-21: Enhanced type safety with explicit return typesAdding explicit
Promise<void>return types to the exported functions improves type safety and makes the API more predictable for consumers.Also applies to: 28-28, 35-35
v3/pkg/application/messageprocessor_call.go (1)
154-156: Improved defer logging patternWrapping the logging call in a deferred anonymous function improves code structure while maintaining the same logging behavior. This approach better encapsulates the deferred operation and improves readability.
v3/tasks/events/generate.go (2)
10-10: Use ofconstfor large multi-line string
DefiningeventsGoas aconstis fine for readability and immutability. However, note that very large multi-line constants can slightly increase compile-time memory usage. If this string grows significantly, consider usingvaror embedding it as a file if memory usage becomes a concern.
288-292: Straightforward TypeScript event file generation
Replacing the placeholders with platform-specific event strings and writing toevent_types.tsis a clean approach. This logic looks correct and aligns with the rest of the generation flow.v3/internal/generator/render/create.go (10)
228-228: Creation function for array types
This addition concisely handles array creation. The usage of%s$Create.Array(%s)%swith theJSCreateWithParamscall is consistent with other code patterns.
231-231: Creation function for map types
Using$Create.Map($Create.Any, ...)is reasonable for map creation, although it treats map keys as$Create.Any. Double-check downstream logic if strict key typing is required.
239-239: New variables for code generation
Introducingcast, argType, returnTypeclarifies the typed function snippet generation. No apparent issues.
248-248: Dynamic function snippet
Embedding...args%sin the function string is a neat approach to handle varied parameters. Just be mindful of any potential injection if user-defined strings can appear inpp.params.
250-250: Reassigning$$createType
Reassigning the symbol$$createType%dat runtime may cause confusion if referenced multiple times. Confirm this redefinition behaves as expected in edge cases.
254-254: String interpolation
Including multiple format placeholders forcast, pp.index, argType, returnTypeis appropriately structured. Looks good.
256-256: Combining generation parameters
Mergingpre, the underlying type’s creation, andpostclarifies how final code is assembled. This approach is consistent with the rest of the generator.
294-294: Nullable pointer creation
$Create.Nullable(%s)elegantly handles pointer types. Good approach for ensuring typed null checks in the generated code.
321-321: Appendingpostfor struct creation
Finalizing the builder output withpostensures consistent function or signature closure. Straightforward, no issues.
326-326: Default to$Create.Any
Falling back to$Create.Anyis a safe catch-all for types lacking a specialized creation handler.v3/internal/runtime/desktop/@wailsio/runtime/src/utils.ts (5)
13-14: Doc comment change
Removing the type annotation from the doc comment is consistent with the newmessage: anysignature. This is a minimal but clear update.
25-31: Browser feature check forMouseEvent#buttons
The newcanTrackButtonsfunction is straightforward. For older browsers lackingMouseEventconstructors, a fallback check might be necessary, but for modern contexts, this is fine.
35-35: Clarified doc reference
Mentioning the MDN link for the AbortSignal-based listener removal is helpful. No issues here.
84-84: DOM load detection
Adding an arrow callback forDOMContentLoadedsetsisReadysuccinctly. This snippet is minimal and functional.
86-86: Type-safe callback forwhenReady
Defining the callback as() => voidclarifies usage. This is an improvement for type safety.v3/internal/runtime/desktop/@wailsio/runtime/src/index.ts (1)
1-58: Well-structured TypeScript entry point with proper exportsThe new index.ts file is well organized and follows modern TypeScript practices. It properly initializes the Wails runtime, imports necessary modules, and exports them as part of the public API.
v3/internal/generator/render/templates/service.js.tmpl (3)
25-25: Updated imports to support cancellable promisesThe template now imports
CancellablePromiseinstead ofCreatewhen not using interfaces, which aligns with the PR objective of implementing cancellable promises for long-running binding calls.
49-59: Return type updated to CancellablePromiseThe return type annotation has been changed from
Promise<...>to$CancellablePromise<...>, properly reflecting the new cancellation capabilities in the type system.
67-73: Simplified promise return patternThe code has been improved by directly returning the result of
$Call.ByNameor$Call.ByIDwithout assigning to an intermediate variable, which makes the code more concise.v3/internal/runtime/desktop/@wailsio/runtime/tsconfig.json (4)
3-3: Added test file exclusionExcluding test files from the production build is a good practice that keeps the distribution cleaner.
5-14: Enhanced TypeScript configuration for better build controlThe configuration now includes important compiler options like
composite,noEmitOnError, and explicit directory settings for declarations and source files. These changes improve the build process reliability.
16-27: Upgraded module settings and standardized library namesThe target and module settings have been upgraded to ES2015, with proper casing for library names (DOM, DOM.Iterable, ESNext). These changes follow TypeScript best practices.
30-34: Added strict type-checking optionsThe additional type-checking options like
noUnusedLocals,noImplicitAny, andnoImplicitReturnswill help catch potential issues during development, improving code quality.docs/src/content/docs/learn/bindings.mdx (5)
440-443: Clarified cancellation behaviorThe documentation now clearly explains that calling the
cancelmethod will reject the promise immediately with a special cancellation error, while also canceling the Go context and discarding the actual result.🧰 Tools
🪛 LanguageTool
[uncategorized] ~440-~440: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...e specialcancelmethod on the promise and it will reject immediately with a speci...(COMMA_COMPOUND_SENTENCE)
453-484: Added comprehensive documentation for cancellable promisesThe documentation now includes detailed examples of the cancellable promise wrapper, showing how cancellation propagates through promise chains, and explaining the optional cause parameter and the behavior of the
cancelmethod.
485-500: Included helpful note about cancellation behaviorThe note about cancellation behavior when a promise has already settled provides important clarity for developers about how the system behaves in edge cases.
502-526: Added AbortController/AbortSignal integrationThe documentation now explains how to use the
cancelOnmethod to tie binding call cancellation to an AbortSignal, which provides a more modern approach for managing asynchronous cancellations.
527-529: Important platform support cautionThe documentation includes a critical caution about AbortSignal support on macOS, noting that it's only supported from macOS 10.15 Catalina onwards, which helps developers avoid compatibility issues.
docs/src/content/docs/learn/runtime.mdx (4)
25-26: Improved terminology: "pre-built bundle" is more precise.The change from "pre-built version of the runtime" to "pre-built bundle" is a good improvement in terminology that more accurately describes what's being provided to developers.
40-53: Enhanced clarity on bundle generation and usage.The updated documentation provides clearer instructions about what the generated runtime bundle is (an ES module) and how it can be used, including the important detail that the API is also exported to the global window object.
58-67: Important script attribute addition and event handling.The addition of the
type="module"attribute to the script tag and the implementation of the window.onload handler are critical changes that ensure proper runtime initialization.
75-91: Valuable new initialization section.The new "Initialisation" section provides essential information about runtime initialization, especially for context menus and window dragging functionality. The note about side-effect imports is particularly helpful for bundler configurations.
Minor spelling note: In American English, "side effect" and "side effects" are typically not hyphenated.
🧰 Tools
🪛 LanguageTool
[misspelling] ~79-~79: Did you mean “side effect” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...n't use the API, make sure to include a side-effect import statement somewhere in your fron...(AFFECT_EFFECT)
[misspelling] ~86-~86: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...r bundler should detect the presence of side-effects and include all required initialisation...(AFFECT_EFFECT)
v3/examples/binding/assets/bindings/github.com/wailsapp/wails/v3/examples/binding/greetservice.js (3)
12-12: Updated imports to use CancellablePromise.The import has been updated to include the new
CancellablePromisetype from the Wails runtime, aligning with the PR's feature of cancellable promises.
21-21: Return type updated to CancellablePromise.The return type annotations have been updated from
Promiseto$CancellablePromise, aligning with the PR description's mention of cancellable promises for long-running binding calls.This change supports the PR objective of improved cancellable promises. Note that as mentioned in the PR description, this introduces a breaking change where cancelled binding calls will now reject with a
CancelErrorexception instead of returning a value.Also applies to: 33-33, 42-42
24-27: Simplified function implementations.The implementation has been simplified to directly return the result of
$Call.ByID, which is now a cancellable promise. This reduces code complexity while maintaining the same functionality.Also applies to: 36-37, 45-46
v3/internal/runtime/desktop/@wailsio/runtime/src/listener.ts (4)
14-14: Well-typed event listeners map.The
eventListenersmap provides a type-safe way to store and retrieve event listeners by their event name.
16-38: Well-designed Listener class with error handling.The
Listenerclass implementation provides:
- Clean encapsulation of listener properties
- A maxCallbacks parameter with sensible default (-1 for unlimited)
- Error handling during callback execution
- Logic to track and limit the number of invocations
This is a robust implementation that handles edge cases appropriately.
27-37: Verify error handling approach.The current error handling logic catches and logs errors without propagating them. Consider whether this is the desired behavior for all use cases.
Should errors in event listeners be silently caught and logged, or should there be a way to configure error handling behavior? This could potentially hide bugs in event handlers.
40-52: Efficient listener removal implementation.The
listenerOfffunction efficiently removes listeners while maintaining map integrity:
- Properly handles the case when no listeners exist
- Uses filter to remove the specific listener instance
- Cleans up the map when no listeners remain for an event
- Updates the map with the filtered list otherwise
This prevents memory leaks and ensures proper event listener lifecycle management.
v3/internal/runtime/desktop/@wailsio/runtime/src/drag.ts (6)
15-27: Well-organized state management.The global state variables are logically organized by functionality (dragging vs. resizing) with clear naming that indicates their purpose. The use of the
canTrackButtons()utility function helps with cross-browser compatibility.
28-36: Properly implemented window.setResizable method.The
setResizablemethod:
- Updates the resizable state
- Properly cleans up resizing state when resizable is set to false
- Updates the cursor back to default when needed
This implementation ensures consistent behavior when window resizability changes.
55-115: Complex event synthesis requires careful testing.The event synthesis logic in the
updatefunction is quite complex, especially the handling of button state tracking and the synthesis of release-press sequences. This approach is necessary for cross-platform compatibility but increases the risk of edge-case bugs.Ensure this has been thoroughly tested across different browsers and operating systems, particularly on Windows where the comment mentions that "Windows suppresses mouse events at the end of dragging or resizing."
117-157: Thorough edge case handling in primaryDown.The
primaryDownfunction:
- Handles platform-specific behavior differences (Windows vs. macOS/Linux)
- Correctly prioritizes resize over drag when on resize edges
- Safely handles different types of event targets
- Properly checks for draggable elements using CSS custom properties
- Includes click-on-scrollbar detection
This comprehensive approach should provide a smooth user experience across platforms.
167-176: Good use of Object.freeze for cursor mapping.The
cursorForEdgeobject is properly frozen to prevent accidental modification, and it maps resize edge identifiers to the appropriate cursor styles.
189-245: Sophisticated resize edge detection.The
onMouseMovefunction implements detailed logic for:
- Starting drag or resize operations when ready
- Detecting window borders and corners with configurable sizes
- Applying the appropriate cursor based on the detected edge
- Includes an optimization for the case when the cursor is outside all corner areas
The use of configuration flags for resize handle dimensions adds flexibility for different themes and platforms.
v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/kvstore/keyvaluestore.js (3)
7-7: Import updated to include CancellablePromiseThe import statement has been updated to include the
CancellablePromisetype, which aligns with the PR's goal of enhancing cancellation capabilities for promises.
12-12: Return types updated to use cancellable promisesThe return type annotations have been updated to use
$CancellablePromise<void>or$CancellablePromise<any>instead of the previousPromise<void> & { cancel(): void }pattern. This is a cleaner approach that leverages the new cancellable promises implementation.Also applies to: 21-21, 29-29, 39-39
15-15: Function implementation simplifiedThe implementation has been simplified to directly return the result of
$Call.ByIDwithout storing it in an intermediate variable. This makes the code more concise while maintaining the same functionality.Also applies to: 24-24, 32-32, 42-42
v3/internal/runtime/desktop/@wailsio/runtime/src/callable.ts (4)
1-24: Well-documented license and attributionThe file properly includes the original MIT license and attributes the source, which is good practice for code reuse.
25-46: Robust fallback mechanism for Reflect.applyThe code implements a thorough mechanism to check for
Reflect.applysupport and provides a fallback option. This increases compatibility across different JavaScript environments.
102-112: Well-implemented callable detection functionsBoth
isCallableRefApplyandisCallableNoRefApplyfunctions provide thorough checks for callable objects, handling edge cases like ES6 classes and Document Dot All (DDA) types. The typings using TypeScript's type predicates (value is (...args: any[]) => any) ensure proper type narrowing when used.Also applies to: 114-123
125-125: Clever export based on environment capabilityThe default export intelligently selects the most appropriate implementation based on the environment's capabilities, which is an elegant design pattern.
.github/workflows/build-and-test-v3.yml (4)
25-68: New job for checking runtime build artifactsAdding a dedicated job to check for unwanted changes to build artifacts is an excellent practice. This ensures that developers don't accidentally commit generated files and that the CI process builds them consistently.
The error message provides clear instructions for how to revert artifact changes, which will be helpful for contributors.
103-103: Standardized working directory pathsWorking directory paths have been standardized from
./v3to justv3, which improves consistency throughout the workflow file.Also applies to: 110-110, 115-115, 120-120, 129-129
149-176: Enhanced JavaScript testing stepsThe additional steps for cleaning build artifacts, type-checking, testing the runtime, and verifying that the npm package builds correctly strengthen the CI process specifically for TypeScript changes. This is especially important for a PR that's migrating from JavaScript to TypeScript.
223-223: Consistent working directory updateThis path standardization maintains consistency with the other changes in the file.
v3/internal/runtime/desktop/@wailsio/runtime/src/events.test.js (5)
1-2: Updated imports and event dispatch accessThe imports have been updated, and
dispatchWailsEventis now accessed directly fromwindow._wailsinstead of being imported from the events module. This reflects architectural changes in how events are handled in the TypeScript runtime.Also applies to: 5-5
12-45: Improved OnMultiple testsThe
OnMultipletest suite has been restructured and enhanced with:
- A dedicated test for proper
WailsEventinitialization- More specific assertions using
toHaveBeenCalledTimesinstead oftoBeCalledTimes- Better test for the cancel functionality
These changes improve test readability and correctness.
47-74: Enhanced On tests with maxCallbacks verificationThe
Ontest suite now includes a test that verifies the listener never stops by checking thatmaxCallbacksremains-1. This is an important invariant to test and ensures the behavior matches expectations.
76-103: Improved Once testsSimilar to the other test suites, the
Oncetests have been restructured to be more specific and comprehensive, clearly verifying that callbacks are only called once and that cancellation works properly.
105-136: More comprehensive Off testsThe
Offtests now cover both single and multiple event type cancellation scenarios, with clear expectations for each callback's invocation count. This provides better test coverage for the event cancellation functionality.v3/internal/runtime/desktop/@wailsio/runtime/src/cancellable.test.js (4)
19-46: Ensure reliability of the global unhandled rejection handler for parallel test runs.Attaching a global
unhandledRejectionlistener can introduce race conditions in environments that run tests in parallel or overlap. Consider using a per-test mechanism for capturing unhandled rejections, or ensuring the tests run sequentially to avoid polluting the global environment.
136-146: Great approach to verifying cancellation errors remain unreported.This test effectively verifies that cancellation does not inadvertently trigger unhandled rejections. The
sentinelusage is clear and ensures the correct outcome.
148-161: Verify the correct handling of arbitrary rejections.This test ensures that only cancellation errors are suppressed. It’s robust and distinguishes between canceled tasks and genuinely unhandled rejections.
244-287: Potential timing flakiness with setTimeout-based abort tests.If the environment’s timing is unreliable or if test durations vary, these abort patterns may occasionally produce race conditions. Confirm that test environments consistently handle the 25ms timeout or consider adjustable/faster timeouts for more deterministic outcomes.
v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts (1)
38-42: Good separation of concerns with a higher-order function.
newRuntimeCallernicely abstracts object-level invocation logic. It helps keep your code DRY and easy to extend. No issues found.v3/internal/generator/render/templates/service.ts.tmpl (6)
24-24: Use of the CancellablePromise import is consistent and clear.
This import line correctly introduces theCancellablePromisetype for cancellable asynchronous operations.
52-52: Return type updated to CancellablePromise.
Ensuring all service methods now return aCancellablePromisealigns with the new cancellation design. Consider verifying downstream usage to confirm that all call sites handle cancellation correctly.
62-62: Braced type closure is well-formed.
This closing brace completes theCancellablePromisegeneric definition. No issues here.
64-64: Directly returning ByName or ByID calls is a clean approach.
Removing intermediate variables simplifies the code. This aligns well with the updated asynchronous approach, keeping the code concise.Also applies to: 66-66
68-71: Conditional promise chain logic.
Switching between a direct return or a.thenblock based on$useInterfaces, result presence, or$module.SkipCreateis an elegant solution. Confirm that all relevant call sites handle both scenarios (with or without post-processing) as intended.
82-82: Final promise resolution closure.
The terminating});properly concludes the.thenblock for post-processing results. No concerns.v3/internal/runtime/desktop/@wailsio/runtime/src/calls.ts (12)
1-9: License header and high-level file banner.
This provides a clear legal and contextual overview. No specific issues.
11-14: Imports for core functionality and nanoid.
Imports ofCancellablePromise,CancellablePromiseWithResolvers,nanoid, and local runtime references are consistent with the new cancellation design. Good separation of concerns.
20-21: PromiseResolvers type.
The omission of thepromiseandoncancelledfields is a practical approach for an internal resolver structure. This preserves clarity while exposing only essential methods to the rest of the code.
22-25: Creation of call-related variables.
call,cancelCall, andcallResponsesare fundamental building blocks for managing requests. TheMap<string, PromiseResolvers>is suitable for storing resolution and rejection callbacks keyed by unique IDs.
29-47:CallOptionstype with methodID or methodName.
This union type enforces mutual exclusivity ofmethodIDandmethodName, which improves type safety. It’s clear and strongly typed.
49-63:RuntimeErrorclass.
Defining a specialized runtime error class helps differentiate binding-related errors. The use ofthis.name = "RuntimeError";is appropriate for error identification.
65-89:resultHandlerfor call success.
Gracefully handles empty data, raw strings, and JSON. Thetry/catchblock for JSON parsing is well-handled, though logging or surfacing parse errors might help debugging.
91-138:errorHandlerfor call failures.
Rejecting the promise with the correct error class (ReferenceError,TypeError,RuntimeError, orError) is robust. Consider logging the raw error data if feasible, to simplify debugging.
140-150:getAndDeleteResponseutility.
Encapsulating retrieval and cleanup in a single function reduces duplication and possible memory leaks. Straightforward usage of theMap.
152-163:generateIDwith nanoid and duplication check.
Ensuring each call ID is unique by looping until a new ID is created is correct. The chance of collisions withnanoidis low, but the extra check is protective.
211-221:ByNamewrapper for calls.
Straightforward convenience function for calling a bound method by fully-qualified name. The rest of the logic delegates toCall.
223-233:ByIDwrapper for calls.
MirrorsByNamebut uses the numeric ID approach. This consistent pattern fosters clarity for different invocation methods.docs/src/content/docs/changelog.mdx (3)
63-65: New cancellable promise features and binding call cancellation with AbortSignal.
These additions highlight advanced asynchronous control. Ensure the documentation includes examples on how to integrate the newCancellablePromiseandAbortSignalusage.
96-96: Improved dragging and resizing mechanism for cross-platform consistency.
This is a great quality-of-life improvement. Validate any new behaviors against OS-specific guidelines to ensure an intuitive user experience.
117-118: TypeScript port of the JS runtime and CancellablePromise for bound methods.
These breaking changes are well-documented. Confirm that existing user code is updated accordingly and that users are aware ofCancelErrorhandling.v3/internal/runtime/Taskfile.yaml (4)
14-19: Task renamed and updated for clarityThe task previously named
testhas been renamed tocheckwith the command changed tonpm run check. This better reflects its purpose of running basic verification checks rather than full tests.
21-26: Task organization improvedThe task previously named
updatehas been appropriately renamed totest, which now runs the actual test suite vianpm test. This makes the naming more intuitive and consistent with standard conventions.
31-31: Updated build target for TypeScript migrationThe build command has been updated to use TypeScript source files (
.tsinstead of.js) with ES2015 target and appropriate flags for the TypeScript migration.
36-36: Production build updated for TypeScriptSimilar to the debug build, the production build has been updated to use TypeScript source files while maintaining minification and console removal for production environments.
v3/internal/runtime/desktop/@wailsio/runtime/src/screens.ts (6)
11-16: Well-defined Size interface with proper documentationThe
Sizeinterface is well-defined with clear documentation for each property, providing good type safety for width and height dimensions.
18-27: Well-defined Rect interface with proper documentationThe
Rectinterface appropriately extends the concept ofSizeby adding position coordinates (X, Y) with clear documentation for each property.
29-54: Comprehensive Screen interface with detailed documentationThe
Screeninterface is well-designed with thorough documentation for each property. It covers all necessary aspects of screen information including identification, dimensions, scale factors, and work areas.
68-70: Clean API for retrieving all screensThe
GetAllfunction provides a simple and clean API for retrieving all screens, with proper TypeScript return type annotation.
77-79: Clean API for retrieving primary screenThe
GetPrimaryfunction provides a simple and clean API for retrieving the primary screen, with proper TypeScript return type annotation.
86-88: Clean API for retrieving current screenThe
GetCurrentfunction provides a simple and clean API for retrieving the current active screen, with proper TypeScript return type annotation.v3/internal/runtime/desktop/@wailsio/runtime/src/events.ts (9)
26-26: Clear callback type definitionThe
Callbacktype is well-defined to accept aWailsEventparameter, providing proper type checking for event handlers.
31-52: Well-structured WailsEvent classThe
WailsEventclass is well-structured with appropriate properties and documentation. Properties are clearly typed, and the constructor properly initializes the event.
54-71: Efficient event dispatching implementationThe
dispatchWailsEventfunction efficiently handles event dispatching by:
- Retrieving listeners for the event
- Creating a proper
WailsEventinstance- Managing listener lifecycle based on dispatch results
- Cleaning up empty listener collections
The implementation maintains good performance while ensuring proper event delivery.
81-87: Flexible OnMultiple registration with cleanupThe
OnMultiplefunction provides a flexible way to register event handlers with count-limited execution. It returns a cleanup function that removes the listener when called, which is a good practice for preventing memory leaks.
96-98: Clean unlimited event registrationThe
Onfunction provides a simple API for unlimited event registration by leveragingOnMultiplewith a -1 max count. This is a good example of code reuse.
107-109: Clean single-execution event registrationThe
Oncefunction provides a simple API for single-execution event registration by leveragingOnMultiplewith a max count of 1. This maintains consistency with the other registration methods.
116-118: Efficient event listener removalThe
Offfunction effectively removes listeners for specified events using rest parameters for a flexible API. The implementation is clean and efficient.
123-125: Simple global listener cleanupThe
OffAllfunction provides a straightforward way to remove all event listeners, which is important for complete cleanup scenarios.
133-135: Type-safe event emissionThe
Emitfunction accepts aWailsEventinstance, ensuring type safety when emitting events. The implementation is clean and straightforward.v3/examples/services/assets/bindings/github.com/wailsapp/wails/v3/pkg/services/sqlite/service.js (5)
7-7: Updated imports for cancellable promisesThe import statement has been updated to include
CancellablePromise, aligning with the PR objective of implementing cancellable promises for improved handling of long-running binding calls.
10-14: Updated return type and simplified implementationThe
Closefunction now correctly specifies$CancellablePromise<string>as its return type, and the implementation has been simplified by directly returning the result of$Call.ByID. This change aligns with the "cancellable promises" enhancement mentioned in the PR objectives.
19-23: Updated return type and simplified implementation for ExecuteThe
Executefunction now correctly specifies$CancellablePromise<void>as its return type, and the implementation has been simplified by directly returning the result of$Call.ByID. This improves code readability while maintaining the same functionality.
27-31: Updated return type and simplified implementation for OpenThe
Openfunction now correctly specifies$CancellablePromise<string>as its return type, and the implementation has been simplified by directly returning the result of$Call.ByID. This change improves code readability and maintainability.
36-41: Updated return type and simplified chaining for SelectThe
Selectfunction now correctly specifies$CancellablePromise<{ [_: string]: any }[]>as its return type and uses direct method chaining for processing the result. This change aligns with the TypeScript migration and improves code simplicity..github/workflows/publish-npm.yml (10)
20-25: Installation of Task with repository token looks good.
No issues found.
26-30: Node.js version usage is straightforward.
No issues found.🧰 Tools
🪛 actionlint (1.7.4)
27-27: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
31-34: Cleaning build artifacts step
No issues found; cleaning prior builds is a best practice.
35-40: Building the bundled runtime
No issues found. Ensure theesbuildusage is correct for your environment.
41-47: Testing and building npm package
This step looks solid, covering install, test, and prepack.
48-55: Detecting npm package changes
No issues found; verifying changes in dist/types directories is appropriate for release logic.
56-62: Bumping package version
Automatically bumping withnpm version prereleaseis a neat approach.
64-67: Committing changes with [skip ci]
Looks good. Preventing CI loops with[skip ci]is highly recommended.
70-77: Publishing npm package
No issues found; uses a well-known action with the correct configuration.
78-93: Detecting and committing docs/bundled runtime changes
No issues found; the fallback steps for docs and bundled runtime changes are handled cleanly.v3/internal/assetserver/bundledassets/runtime.js (1)
1-2: Double-check for possible switch-case variable declarations.Based on the static analysis hints, ensure that variables declared within switch cases are block-scoped with braces to avoid leaks or re-declarations. Although no direct issue is evident here, it might be worthwhile to confirm that no inadvertent variable scope conflicts occur in these switch statements.
🧰 Tools
🪛 Biome (1.9.4)
[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: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
v3/internal/runtime/desktop/@wailsio/runtime/package.json (4)
7-8: Exports look good; ensure tree-shaking is not disrupted.Switching to
"import": "./dist/index.js"and providing"types": "./types/index.d.ts"is a clean approach, but confirm that bundlers can still tree-shake the code effectively. Some bundlers might rely on additional fields like"module"or"main".
12-13: Repository directory update.Specifying
"directory": "v3/internal/runtime/desktop/@wailsio/runtime"is beneficial for clarity. Just ensure any contributing or publishing scripts rely on this field correctly and that any references to the repository path remain consistent.
21-29: Thank you for specifying files and sideEffects explicitly.Defining
"files"helps ensure only relevant artifacts get published, and"sideEffects"hints to bundlers about which modules might have side effects. This is a good practice that improves package clarity.
54-59: Overrides for 'promises-aplus-tests'.Ensure that the version overrides (Mocha, Sinon, and Underscore) are all compatible with your testing framework (Vitest) and do not cause conflicts in your CI pipeline.
v3/internal/generator/testdata/tsconfig.json (4)
2-3: Explicitly including and excluding files is a solid approach.Your
"include": ["output/**/*.js", "output/**/*.ts"]and"exclude": ["output/**/*.got.?s"]are straightforward. Make sure this aligns with your actual code layout so that no necessary files are inadvertently left out.
7-10: 'noEmit' combined with 'allowJs' and 'skipLibCheck'.You're not emitting anything but still allowing JS files. This is valid if you rely on a separate build tool (e.g., bundler/transpiler). Just confirm that you don't need
.d.tsgeneration here or plan to rely on this config for final builds.
13-17: Confirm 'moduleResolution': 'bundler' for your environment.Using
"moduleResolution": "bundler"and"verbatimModuleSyntax": true"is suitable for certain build pipelines (e.g., ES modules in a bundler). Verify your environment or toolchain can handle this resolution setup without hitting runtime import issues.
31-33: Paths mapping appears correct.Mapping
"/wails/runtime.js"to["../../runtime/desktop/@wailsio/runtime/src/index.ts"]ensures local imports use the TypeScript source. Confirm that your bundler or build scripts do not need additional adjustments for these alias mappings.
[approve]v3/internal/runtime/desktop/@wailsio/runtime/src/dialogs.ts (1)
1-9: Great overall structure and clarity.No specific issues spotted in the header and introductory comments. Code style is consistent and readable here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/publish-npm.yml (2)
27-31: Node.js Version Setup & Action Version Consideration.
The job now sets up Node.js 20 as required. However, static analysis has flagged thatactions/setup-node@v3might be outdated for GitHub Actions. Consider upgrading to a newer version (e.g., v4) to avoid potential runner compatibility issues. For example:- - name: Use Node.js 20 - uses: actions/setup-node@v3 + - name: Use Node.js 20 + uses: actions/setup-node@v4🧰 Tools
🪛 actionlint (1.7.4)
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
64-70: Commit Changes Step.
This step commits and pushes changes (including the new version) when package changes are detected or when manually triggered. As a suggestion for enhanced safety, consider limiting the scope ofgit add .to specific directories (such as thedistandtypesfolders) to avoid unintentional commits.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-npm.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/publish-npm.yml
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests (macos-latest, 1.24)
- GitHub Check: Run Go Tests (windows-latest, 1.24)
🔇 Additional comments (10)
.github/workflows/publish-npm.yml (10)
4-4: Added Manual Workflow Trigger.
Including theworkflow_dispatchtrigger enhances the workflow’s flexibility by allowing manual execution—particularly useful given the pending test data update.
21-26: Updated Task Runner Setup.
The workflow now usesarduino/setup-task@v2with a flexible version specifier (3.x) and supplies therepo-tokeninput. This change improves maintainability and aligns with modern task runner practices.
32-34: Clean Build Artifacts Step.
The newly added step runsnpm run cleanin the specified working directory (v3/internal/runtime/desktop/@wailsio/runtime). Verify that the directory exists and that the clean script effectively removes outdated build artifacts.
36-41: Build Bundled Runtime Step.
This step usesnpxto ensure that the latestesbuildis available and then runs the task runner command (task runtime:build) to bundle the runtime. Please ensure that usingv3as the working directory is the intended behavior and that the build sequence meets your build process requirements.
42-48: Test and Build npm Package Step.
Runningnpm ci,npm test, andnpm run prepackunder the specified working directory effectively ensures that the package is tested and prepackaged correctly before publishing. Confirm that all tests pass on your target platforms.
49-56: Detect npm Package Changes Step.
The updated action now checks for modifications in thedistandtypesdirectories. This improved detection mechanism should better capture meaningful package changes that warrant version bumps and publishing.
57-63: Bump Version Step.
This conditional step invokesnpm version prereleaseto bump the version when triggered manually or when package changes are detected. Ensure that the version output is correctly captured in$GITHUB_OUTPUTfor subsequent steps.
71-78: Publish npm Package Step.
The publishing action usesJS-DevTools/npm-publish@v3to publish the package conditionally. Verify that the provided package path (v3/internal/runtime/desktop/@wailsio/runtime) and the NPM token are correctly configured for secure and successful publishing.
79-87: Detect Docs and Bundled Runtime Changes Step.
This step conditionally detects changes in the documentation and bundled assets directories when no npm package changes are found. This separation ensures that even non-package changes are committed appropriately.
88-93: Commit Docs and Bundled Runtime Changes Step.
This final conditional step commits and pushes documentation and asset bundling changes when they’re detected. Verify that the commit conditions and message align with your project’s release and changelog practices.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-and-test-v3.yml (1)
25-68: [Artifact Change Detection Logic – Review of the “check_build_artifacts” Job]
This new job correctly checks for modifications in the runtime build artifacts. It uses thetj-actions/changed-files@v45action to detect changes, then calculates the merge base before failing the job with a clear error message. One minor suggestion is to simplify the newline insertion in line 64. Instead of using${"\n"}you can directly include
\nin the template literal for clarity and consistency.Suggested diff for line 64:
- ` git restore -s ${process.env.MERGE_BASE_SHA} -W -S -- dist types &&${"\n"}` + + ` git restore -s ${process.env.MERGE_BASE_SHA} -W -S -- dist types &&\n` +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/build-and-test-v3.yml(4 hunks).github/workflows/publish-npm.yml(2 hunks)v3/internal/generator/render/info.go(3 hunks)v3/internal/generator/render/templates/models.js.tmpl(2 hunks)v3/internal/generator/render/templates/models.ts.tmpl(2 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/create.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- v3/internal/generator/render/info.go
- v3/internal/generator/render/templates/models.ts.tmpl
- v3/internal/generator/render/templates/models.js.tmpl
🧰 Additional context used
🪛 Biome (1.9.4)
v3/internal/runtime/desktop/@wailsio/runtime/src/create.ts
[error] 31-31: Do not shadow the global "Array" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 52-52: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests (macos-latest, 1.24)
- GitHub Check: Run Go Tests (windows-latest, 1.24)
🔇 Additional comments (21)
v3/internal/runtime/desktop/@wailsio/runtime/src/create.ts (6)
14-15: Improved type safety with generics - looks good!The addition of the generic type parameter
Tenhances type safety by allowing the function to return a properly typed value while maintaining flexibility.
22-23: Explicit typing improves code clarityThe explicit input and return type annotations make the function's purpose clearer and improve type safety.
31-31: Function overshadows global Array constructorUsing
Arrayas a function name creates a naming collision with the JavaScript globalArrayconstructor, which could lead to confusion for developers unfamiliar with this codebase.While I note from past reviews that you've decided to keep this name due to limited scope and type-checking safeguards, consider renaming it to something like
CreateArrayin a future refactoring to improve code maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: Do not shadow the global "Array" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
52-52: Function overshadows global Map constructorUsing
Mapas a function name creates a naming collision with the JavaScript globalMapconstructor.I see from previous reviews that you've decided to keep this name. As with the
Arrayfunction, consider renaming it to something likeCreateMapin a future refactoring to improve maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
72-72: Well-typed nullable wrapper functionThe generic type parameter and explicit union type (
T | null) clearly communicate this function's purpose and improve type safety.
84-86: Enhanced struct creation with proper generic typingThe improved type annotations make this function more robust by:
- Properly typing the input record
- Using a generic type parameter for the output struct
- Maintaining flexibility with the default
anytypeThis allows for better type inference and validation when creating structured objects.
.github/workflows/build-and-test-v3.yml (4)
69-118: [Test JS Job – Updated Workflow Steps Verification]
Thetest_jsjob now includes several updated steps to clean build artifacts, type-check the runtime, run tests, build the bundled runtime, and finally verify that the npm package builds. The updated working-directory paths (removing the leading./) improve consistency with the rest of the workflow. Overall, the job is well structured and meets the updated requirements.
160-165: [Runtime Artifacts Retrieval in Go Tests]
The addition of the "Retrieve runtime build artifacts" step (lines 160–165) in thetest_gojob ensures that artifacts generated during thetest_jsjob are available for subsequent steps. This is an effective use of artifacts to maintain consistency across jobs.
196-206: [Cleanup Job – Artifact Deletion]
Thecleanupjob is straightforward and uses thegeekyeggo/delete-artifact@v5action to delete runtime build artifacts. The inclusion offailOnError: falseensures that cleanup does not cause job failures, which is a good practice.
230-265: [Test Templates Job – Template Generation Verification]
Thetest_templatesjob that builds and tests various templates is clear and well-structured. It covers multiple frameworks consistently and leverages thewails3commands effectively. No issues are noted here..github/workflows/publish-npm.yml (11)
1-5: [Workflow Triggers – Manual and Push Triggers]
Adding theworkflow_dispatchtrigger alongside the push event for thev3-alphabranch provides flexible manual execution. This aligns well with the PR objectives and improves overall control over the publishing process.
21-26: [Install Task Step Update]
The update to usearduino/setup-task@v2with a flexible version range (3.x) and the addition of therepo-tokeninput improves maintainability and access control. This change is consistent with migrating to a more robust build system.
27-31: [Node.js Setup Update]
The explicit use of Node.js 20 in the “Use Node.js 20” step confirms that the runtime will be built with the correct environment. This is straightforward and meets expectations.
38-41: [Clean Build Artifacts Step]
The "Clean build artifacts" step now properly callsnpm run cleanin the appropriate working directory. This change ensures that no stale build outputs affect the publishing process.
42-51: [Bundled Runtime Build and npm Package Testing]
The steps to build the bundled runtime (usingtask runtime:build) and then run tests/build commands for the npm package are clear. Separating these steps helps ensure that the packaging process only runs if the build is correct.
52-59: [Detection of NPM Package Changes]
Usingtj-actions/verify-changed-files@v20to detect changes in thedistandtypesdirectories effectively ensures that the package version bump and publication only trigger when necessary. The configuration is clear and concise.
60-66: [Automated Version Bumping]
The "Bump version" step, which usesnpm version prereleaseand writes the new version to$GITHUB_OUTPUT, is a practical way to automate version management based on detected changes. This helps streamline the release process.
67-73: [Committing Package Changes]
The commit step for package changes is well guarded by conditions and includes the[skip ci]tag to prevent triggering additional workflows. Using dynamic commit messages that incorporate the new version is a nice touch.
74-81: [NPM Package Publication]
The publication step usesJS-DevTools/npm-publish@v3conditioned on being triggered manually or when package-related changes are detected. The parameters provided (package path, access level, token) are correctly configured.
82-90: [Detection of Docs and Bundled Runtime Changes]
The additional step that detects changes in documentation and bundled runtime assets is a good safeguard. It allows the workflow to handle updates that do not affect the npm package directly but still require a commit.
91-97: [Automated Commit for Docs and Bundled Changes]
The final commit step for documentation and bundled runtime updates is conditioned carefully to run only when appropriate. Including[skip ci]in the commit message is a smart move to prevent unnecessary CI cycles.
|
Fixed a CI issue whereby the binding generator wasn't being tested against the latest version of the runtime. Discovered a typing issue in the binding generator, fixed that too. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/src/content/docs/changelog.mdx (1)
118-121: Review TypeScript Migration and Runtime Initialization UpdatesThe changes in these entries clearly document:
- The migration of the JS runtime to TypeScript,
- Immediate initialization upon import (eliminating the need for a separate init method), and
- The new behavior where bound methods return a
CancellablePromisethat rejects with aCancelErrorif cancelled.Note: The term “side-effects import” on line 120 might be better rendered as “side effects import” (without the hyphen) to align with common documentation style guidelines.
🧰 Tools
🪛 LanguageTool
[misspelling] ~120-~120: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...es not export an init method anymore. A side-effects import can be used to initialise it by ...(AFFECT_EFFECT)
.github/workflows/build-and-test-v3.yml (3)
25-33: New "check_build_artifacts" Job Added
This new job clearly isolates the check for runtime build artifact modifications. The use ofactions/checkout@v4with persisted credentials and a dedicated job name improves clarity and separation of concerns.Suggestion: Consider adding an inline YAML comment above this job to document its purpose for future maintainers.
51-68: Artifact Change Enforcement Using GitHub Script
This step enforces the rejection of pull requests that include changes to runtime build artifacts. The use ofactions/github-script@v7to exit with an error (and log a clear recommendation for reverting) is effective.Suggestion: Consider using
process.exit(1)instead of settingprocess.exitCode = 1for immediate termination if that better suits your error-handling policy. For example:- process.exitCode = 1 + process.exit(1)Be sure to test that the immediate exit does not bypass any essential cleanup if needed.
196-206: Cleanup Job for Build Artifacts
The newcleanupjob, which always runs and depends on bothtest_jsandtest_go, usesgeekyeggo/delete-artifact@v5to remove runtime build artifacts. This cleanup routine ensures a tidy environment for subsequent jobs.Suggestion: If artifact retention for debugging is ever required, consider making this step configurable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-and-test-v3.yml(4 hunks)docs/src/content/docs/changelog.mdx(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[misspelling] ~120-~120: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...es not export an init method anymore. A side-effects import can be used to initialise it by ...
(AFFECT_EFFECT)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests (macos-latest, 1.24)
- GitHub Check: Run Go Tests (windows-latest, 1.24)
🔇 Additional comments (26)
docs/src/content/docs/changelog.mdx (2)
63-65: Clarify New Feature AdditionsThese new changelog entries for:
- a cancellable promise wrapper,
- the ability to tie binding call cancellation to an
AbortSignal, and- support for
data-wml-*attributesare clearly stated. Please ensure that the descriptions match the underlying implementation details and that the documentation elsewhere is updated accordingly for consistency.
96-97: Verify Dragging/Resizing Enhancements and Runtime FixThe improvements to the dragging and resizing mechanism—as well as the fix addressing Webpack/Angular discarding the runtime initialization code—are communicated effectively. Ensure that these improvements are reflected in both the user-facing documentation and any related usage examples.
.github/workflows/build-and-test-v3.yml (24)
34-41: Retrieving Changed Files for Artifacts
The step usingtj-actions/changed-files@v45with a file list covering thedist/**andtypes/**directories is well implemented. This setup will accurately detect any unintended modifications to build artifacts.
42-50: Merge Base Calculation
TheGet merge basestep correctly computes the merge base SHA using the pull request’s base and head SHAs. This value is later passed on for artifact verification.Suggestion: Consider adding error handling or a comment if
git merge-basefails unexpectedly.
69-77: JS Test Job Setup ("test_js")
The job header and initial configuration (including dependency oncheck_approvaland the Node.js matrix of[20.x]) are set up appropriately. This ensures that JavaScript testing runs under the correct node version and only after proper PR approval.
78-97: JS Testing Steps – Checkout, Node Setup, and Dependency Installation
The sequence of steps—from checking out the code, setting up Node.js with the specified version, installing Task viaarduino/setup-task@v2, and then installing dependencies (with an inline check on esbuild’s version)—is clear and robust. The use of explicit working directories (e.g.,v3/internal/runtime/desktop/@wailsio/runtime) helps maintain consistency.
98-101: Clean Build Artifacts Step
Runningnpm run cleanin the relevant directory ensures that leftover build artifacts do not interfere with the tests. This is a standard cleanup task and is implemented correctly.
102-105: Type-Checking the Runtime
The step for type-checking (task runtime:check) using the working directoryv3is concise and leverages the centralized Taskfile. Ensure that the corresponding task is properly defined and returns correct error codes on failure.
106-109: Runtime Testing Step
TheTest runtimestep (usingtask runtime:test) is appropriately configured in thev3directory. This helps catch runtime failures early in the CI process.
110-113: Bundled Runtime Build Verification
Usingtask runtime:buildto verify that the bundled runtime builds ensures that both code and configuration are in sync. This step is clear and performs a necessary validation.
114-117: NPM Package Build Check
The job step to runnpm run buildwithin the runtime’s directory confirms that the npm package can be built correctly. The specified working directory is consistent with the other steps.
118-126: Uploading Runtime Build Artifacts
Storing generated build artifacts usingactions/upload-artifact@v4with explicit paths for thedist/,types/, andtsconfig.tsbuildinfofiles is implemented correctly. This not only supports artifact-based testing in subsequent jobs but also reinforces the check against unintended changes.
127-129: "test_go" Job Dependencies Updated
Adding the dependency on bothcheck_approvalandtest_jsensures that Go tests are only run after the approval check and the successful execution of JS tests. This dependency chain is logical and strengthens the CI process.
130-136: Matrix Configuration for Go Tests
The matrix for thetest_gojob now spans multiple operating systems and explicitly sets the Go version. This provides comprehensive cross-platform validation, which is in line with the PR objectives.
160-165: Retrieving Runtime Build Artifacts in Go Tests
Downloading the previously generated runtime artifacts usingactions/download-artifact@v4into the correct directory ensures that the Go tests have access to the necessary runtime components.
166-169: Building Examples in Go Testing
The step to build examples (task test:examples) within thev3directory is a useful addition to validate end-to-end functionality.
170-176: Running Tests on macOS
The macOS-specific test step includes the necessary environment variable (CGO_LDFLAGS) to ensure compatibility. This configuration is precise and adheres to platform-specific requirements.
177-181: Running Tests on Windows
The Windows test step uses a simplego test -v ./...command in the correct working directory. This minimalistic setup is appropriate for the Windows environment.
182-191: Running Tests on Ubuntu with xvfb-run
This step leveragesxvfb-runto simulate an X server for UI-related tests on Ubuntu. The multi-line shell command structure (including a call todbus-update-activation-environment) is suitably organized.Note: Ensure that
xvfb-runis available onubuntu-latestimages and that the multi-line command is parsed correctly.
192-195: Generator Output Typecheck Step
Runningtask generator:test:checkwithin thev3directory validates the output of the binding generator. This step increases confidence in the auto-generated code quality.
207-229: Test Templates Job Matrix Configuration
Thetest_templatesjob now employs a matrix that spans several template types (svelte, vue, react, etc.) alongside multiple operating systems and a specified Go version. This broad testing scope is excellent for cross-technology validation and ensures robustness of template generation.
233-239: Linux Dependencies for Templates
The step for installing Linux dependencies in the test templates job is consistent with previous similar steps, ensuring a uniform environment across jobs.
240-245: Go Environment Setup in Templates Job
Setting up Go with caching (viav3/go.sum) aids in faster builds and guarantees version consistency. This step fits well within the overall multi-job pipeline structure.
246-251: Task Installation for Templates Job
The installation of Task usingarduino/setup-task@v2in the test templates job is in line with the setup steps elsewhere in the workflow, maintaining consistency.
252-257: Building Wails3 CLI within Test Templates
The step that builds the Wails3 CLI (viatask installfollowed bywails3 doctor) is a solid checkpoint to ensure that the CLI tool functions correctly after recent changes.
258-265: Template Generation and Build Verification
The final step creates a new directory for each template, initializes it withwails3 init, and builds it usingwails3 build. This effectively exercises the template generation pipeline.Suggestion: If the CI runners ever handle concurrent jobs on the same workspace, consider using uniquely generated temporary directories to avoid any name collisions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v3/internal/assetserver/bundledassets/runtime.js (1)
1-2: Minified code should have a source map reference.The file appears to be minified JavaScript without an accompanying source map reference. This makes debugging more difficult in production environments.
Consider adding a source map reference using a comment like:
+//# sourceMappingURL=runtime.js.mapThis will help with debugging in production environments when errors occur.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/src/content/docs/changelog.mdx(3 hunks)v3/internal/assetserver/bundledassets/runtime.js(1 hunks)v3/internal/runtime/Taskfile.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- v3/internal/runtime/Taskfile.yaml
- docs/src/content/docs/changelog.mdx
🧰 Additional context used
🪛 Biome (1.9.4)
v3/internal/assetserver/bundledassets/runtime.js
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
🔇 Additional comments (6)
v3/internal/assetserver/bundledassets/runtime.js (6)
54-56: Good implementation of CancelError class.The
CancelErrorclass properly extends the standard Error class with an appropriate name property, following best practices for custom error types.
57-61: Well-structured CancelledRejectionError class.The
CancelledRejectionErrorimplementation properly captures the original promise and reason, with a descriptive error message that includes the original rejection reason.
124-278: Comprehensive CancellablePromise implementation.The
CancellablePromiseclass provides a robust implementation that:
- Extends the native Promise class
- Implements proper cancellation logic
- Handles promise chaining correctly via
then,catch, andfinally- Provides static utility methods like
all,race,any,allSettled- Includes helper methods like
cancel,cancelOn,timeout, andsleepThis implementation aligns with the PR's goal of enhancing cancellation for long-running binding calls.
342-345: Proper handling of both attribute forms.The code correctly supports both
wml-*anddata-wml-*attribute forms as specified in the PR objectives.
575-575: Runtime initializes immediately upon import.The code properly initializes the runtime by sending the "wails:runtime:ready" event immediately, rather than waiting for window load events. This aligns with the PR's goal of immediate initialization.
1-1:Details
✅ Verification successful
Verify cross-platform compatibility of the drag handling.
The drag handling implementation uses platform-specific checks (e.g.,
V()which checks for Windows) and implements different behaviors based on the platform, which aligns with the PR goal of improving drag and resize mechanisms.
🏁 Script executed:
#!/bin/bash # Search for platform-specific drag & resize handling code echo "Platform-specific drag/resize handling:" rg --no-filename "V\(\)" -A 3 -B 3 | head -n 20Length of output: 33288
Cross-platform drag handling verified
The current implementation correctly distinguishes between Windows and non‑Windows environments by using platform-specific checks (e.g., the V() function) and tailored event handling, ensuring that drag and resize behaviors work as intended across platforms.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
# Conflicts: # docs/src/content/docs/changelog.mdx
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/src/content/docs/changelog.mdx(3 hunks)v3/internal/runtime/desktop/@wailsio/runtime/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/content/docs/changelog.mdx
🔇 Additional comments (8)
v3/internal/runtime/desktop/@wailsio/runtime/package.json (8)
8-9: Updated ESM Exports Structure.
The exports field now directly provides the TypeScript types and ESM import path, simplifying the configuration and removing the CommonJSrequireentry. Please verify that all consuming modules (if any) rely on ESM imports only.
13-14: Enhanced Repository Metadata.
The addition of the"directory"field in the repository section helps pinpoint the module’s location within a monorepo. The URL using thegit+https://scheme appears correct.
18-18: Homepage URL Update.
The homepage now points tohttps://v3.wails.io, which aligns with the updated documentation site.
22-25: Defined Package Files for Publication.
Including the./distand./typesdirectories in thefilesfield ensures that only the necessary build artifacts are published with the package.
26-30: Declared Side Effects.
Specifying side effects for key files (e.g.,index.js,contextmenu.js, anddrag.js) helps bundlers understand that these files must be included during tree-shaking. Please update this list if additional modules introduce side effects in the future.
35-36: Task Integration for Event Generation.
Using the commandtask generate:eventsfor event generation is a good move towards a unified, cross-platform build system. Ensure that the accompanying Taskfile is updated to reflect these changes.
45-53: Updated Development Dependencies.
The devDependencies have been updated with modern versions (e.g., TypeScript, Vitest, Happy DOM) and new entries that support the enhanced build/test workflows. Ensure that team members update their environments accordingly.
55-60: Dependency Overrides in Use.
The overrides section ensures that specific versions of inner dependencies used bypromises-aplus-tests(namely mocha, sinon, and underscore) are enforced. Please verify that these versions remain compatible with the rest of the testing infrastructure.
| "build:code": "npx tsc", | ||
| "build:docs": "npx typedoc --plugin typedoc-plugin-mdn-links --plugin typedoc-plugin-missing-exports ./src/index.ts", | ||
| "build:docs:md": "npx typedoc --plugin typedoc-plugin-markdown --plugin typedoc-plugin-mdn-links --plugin typedoc-plugin-missing-exports ./src/index.ts", | ||
| "build": "npm run build:code & npm run build:docs & wait", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cross-Platform Build Script Concern.
The build script on line 41 uses Unix-specific shell operators (& and wait), which may cause compatibility issues on Windows systems. Consider replacing these operators either by using a cross-platform tool (e.g., npm-run-all) or by migrating all build tasks into the Taskfile as discussed in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/build-and-test-v3.yml (1)
276-282: 🛠️ Refactor suggestion
⚠️ Potential issueResults Job Dependency Reference Correction
In theresultsjob, the script referencesneeds.build.result, but there is no job explicitly namedbuildin the dependency list. This is likely a bug that may lead to misinterpreting the combined outcome of the required jobs. Consider replacing this logic with one that aggregates the results fromtest_go,test_js, andtest_templates. For example:- result="${{ needs.build.result }}" - if [[ $result == "success" || $result == "skipped" ]]; then - exit 0 - else - exit 1 - fi + if [[ "${{ needs.test_go.result }}" == "success" && "${{ needs.test_js.result }}" == "success" && "${{ needs.test_templates.result }}" == "success" ]]; then + exit 0 + else + exit 1 + fiThis change ensures that the results job truly reflects the status of all dependent tests.
🧰 Tools
🪛 actionlint (1.7.4)
276-276: property "build" is not defined in object type {test_go: {outputs: {}; result: string}; test_js: {outputs: {}; result: string}; test_templates: {outputs: {}; result: string}}
(expression)
🪛 YAMLlint (1.35.1)
[error] 282-282: no new line character at the end of file
(new-line-at-end-of-file)
♻️ Duplicate comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/package.json (1)
31-43: 🛠️ Refactor suggestionBuild Script Cross-Platform Concern: Address Unix-specific Operators
Thebuildscript on line 41 currently uses Unix-specific shell operators (&andwait), which can cause issues on Windows. This concern was raised in earlier reviews, so to avoid duplication, please consider using a cross-platform solution—such as replacing the script with an npm package likenpm-run-allor migrating the build logic entirely into your Taskfile. For example, one possible diff is:- "build": "npm run build:code & npm run build:docs & wait", + "build": "npm-run-all --parallel build:code build:docs",This change would improve cross-platform compatibility without increasing script complexity.
🧹 Nitpick comments (1)
.github/workflows/build-and-test-v3.yml (1)
90-95: Task Installation Consistency: Confirm Taskfile Migration
The step usingarduino/setup-task@v2to install Task is appropriate given the ongoing migration to using Task-based commands. As discussed in the PR objectives, please ensure that this integration is consistent with your plans to move out of Unix-specific npm scripts entirely.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
v3/internal/runtime/desktop/@wailsio/runtime/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.github/workflows/build-and-test-v3.yml(4 hunks)docs/src/content/docs/changelog.mdx(3 hunks)v3/internal/runtime/desktop/@wailsio/runtime/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/content/docs/changelog.mdx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
🔇 Additional comments (6)
.github/workflows/build-and-test-v3.yml (2)
29-72: Artifact Change Check Job: Verify Build Artifact Safeguard
The newcheck_build_artifactsjob is well structured: it checks for changes in runtime build artifacts, computes the merge base when changes exist, and uses a clear error message to prompt the user to revert artifacts. Please ensure that the file paths used (underv3/internal/runtime/desktop/@wailsio/runtime/dist/**andv3/internal/runtime/desktop/@wailsio/runtime/types/**) remain accurate even as the repository structure evolves.
164-169: Retrieve Build Artifacts: Validate Download Path
TheRetrieve runtime build artifactsstep correctly downloads artifacts into the runtime directory. Verify that the specified path (v3/internal/runtime/desktop/@wailsio/runtime/) is the desired target folder and that it will integrate seamlessly with subsequent steps consuming these artifacts.v3/internal/runtime/desktop/@wailsio/runtime/package.json (4)
7-10: Exports Field Update: Confirm Module Pathing
The modifications in theexportsfield now directly specify thetypesandimportpaths. This simplification aligns well with the new module structure; please double-check that all import paths in consuming projects have been updated accordingly.
11-15: Repository Field Enhancement
The inclusion of thedirectoryfield under therepositorysection is a welcome improvement that clearly indicates the package’s location within the repository.
22-30: Files and SideEffects: Package Inclusion Assurance
Both thefilesandsideEffectsarrays correctly list the directories and files to be published. This ensures only the necessary build outputs are included in the npm package.
44-61: DevDependencies and Overrides: Dependency Alignment
The updates in thedevDependenciesand the introduction of theoverridesfield are well executed. The specified versions appear to align with current requirements.
…ipt-runtime [v3] TypeScript runtime
Description
This PR is ready for review but requires a test data update before merging. Test failures are expected! Passing test results for the complete patch are available at fbbdev/wails#5. Ideally #4068 should be merged before the test data updates on this one.
This PR migrates the JS runtime package to TypeScript. Additionally, it brings:
wml-*attributes alongside thedata-wml-*form preferred by some frontend frameworks;This patch deals mainly with implementation details. Great effort has been put into making the change entirely NON-BREAKING. It should be transparent for all current downstream users, except for the following details:
initmethod anymore: performing animport "@wailsio/runtime"is enough to initialise it exactly once (the ES module system will deduplicate imports). Special care has been taken to ensure and verify that bundlers do not drop the initialisation code.CancelErrorexception.This approach is more in line with the cancellation behaviour of the
fetchAPI, but can be reverted if desired.Any other breakage should be considered as a bug.
Fixes #4097
Type of change
Please select the option that is relevant.
How Has This Been Tested?
The code has been tested extensively both manually and automatically on macOS, Windows and Linux. Examples have been updated and verified to work correctly. New examples have been added.
The updated CI pipelines have been tested on my personal repository, except for the publishing pipeline.
Relevant examples: bindings, cancel-async, cancel-chaining, contextmenu, frameless, services.
Test Configuration
macOS
Windows
Linux
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation