Repository navigation
fix(classic): reach members an interface inherits from its required interfaces - #31
Merged
Merged
Conversation
…nterfaces An IAsyncOperation<T> wrapper on the classic engine only had the members IAsyncOperation declares, so IAsyncInfo's Status, Cancel() and ErrorCode read as undefined. The napi engines already walk the interface tree. Interface wrappers now install the members of required interfaces on the prototype too, each called through its own IID, with the interface's own members taking priority. The parent members installed before them skip every name in the tree, so no prototype slot is added twice. Adds a winrt-async spec to the test app and NSWinRT.toPromise checks to the napi async test: results, rejection, operations staying non-thenable (a promise keeps the operation itself), and the inherited IAsyncInfo members.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On the classic engine, an interface wrapper only had the members its own interface declares. An
IAsyncOperation<T>therefore readStatus,ErrorCodeandCancel()(all declared onIAsyncInfo, which it requires) asundefined. The napi engines already resolve members across the interface tree.NSWinRT.toPromisekept working because it falls back to theCompletedhandler whenStatusis unreadable.Changes
runtime/src/lib.rs:install_interface_prototype_membersnow also installs the members of every required interface, recursively, each called through that interface's own IID and type arguments. The interface's own members take priority over inherited ones with the same name.Tests
test-app/app/tests/winrt-async.js:NSWinRT.toPromiseonIAsyncOperation<T>andIAsyncAction(result,undefined, rejection). Operations are not thenable, soawait Promise.resolve(op)is the operation itself.StatusandCancelare readable, andStatusis1once the operation completes.windows-napi/test/async-test.js: the same checks on the napi path.Results:
pwsh test-app/run.ps1(classic): 89 passed, 0 failed. Before the fix, theStatusspec failed withExpected "undefined" to be "number".windows-napinpm test: 427 passed, 0 failed across 21 suites.