-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Migrate package/flutter to JS static interop. #111315
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
Migrate package/flutter to JS static interop. #111315
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
789c222 to
ce2769f
Compare
1bc3c0f to
e97cf4d
Compare
|
if you rebase upstream to pick up 9ba3581 this diff will be a lot smaller |
e97cf4d to
bf8b98a
Compare
|
@christopherfujino Thanks for the heads up! Much better. |
|
@ditman @eyebrowsoffire ptal. Thoughts? We can consolidate the shims into a single file, I just wasn't sure where it should live. |
eyebrowsoffire
left a comment
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.
Looks good! What's the plan and timing on consolidating a lot of these static JS declarations for browser APIs that are duplicated in the engine/framework/test package? These are eventually going to be directly in the dart SDK right?
|
One update here, I spoke with @eyebrowsoffire offline about the timeline here. There is a lot still up in the air unfortunately. Hopefully we will know more in a few months. |
ditman
left a comment
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.
Let's do it! A few comments/nitpicks below, I don't think there's anything blocking though :)
bf8b98a to
5298d40
Compare
|
test-exempt: code refactor with no semantic change |
687b3b1 to
a4940bd
Compare
This reverts commit 6906493.
This CL migrates a number of Flutter classes to JS static interop. There is still work to be done to migrate the rest of the framework.