-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HybridWebView] Refactor and convert the HybridWebView.js file into a TypeScript file #27183
base: main
Are you sure you want to change the base?
Conversation
bc3d0eb
to
b4ff555
Compare
@MackinnonBuck I hear you are a super TS guru so I was wondering if you had some time to review this refactored code and let me know where I have done off the rails. |
45919c9
to
cfa5963
Compare
5b4193f
to
1e26742
Compare
e125fd3
to
3a008dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TS changes look good to me :) Just a few small comments.
Thanks for the review! |
c041b8e
to
5dd874f
Compare
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
JS is not C++
5dd874f
to
c803e99
Compare
@@ -0,0 +1,255 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noob question, but why do we have both the HybridWebView.js and the HybridView.ts file? Aren't they essentially the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .ts file is a "source file" that gets compiled into .js, and then the .js file is used.
I technically should not be including the js file since it is a build artifact, but just like we keep the .aar file in to avoid failing builds where the tooling is not installed. The TypeScript compiler uses nodejs, so if you don't have that the build will fail.
I am also keeping the .js file for reference/download source since we do not ship the file yet. See #24730
Description of Change
This PR forms part of the effort to make the JS file injected into the app at runtime like the way Blazor works: #24730. However, this PR does not inject anything at this point, just compiles the TS file to JS as part of the build.
This PR only changes a few things:
Issues Fixed
Related to #24730
Fixes #27258