Skip to content

Conversation

@joshualitt
Copy link
Contributor

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.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 9, 2022
@flutter-dashboard
Copy link

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.

@joshualitt joshualitt force-pushed the flutter-changes-for-wasm branch 3 times, most recently from 789c222 to ce2769f Compare September 10, 2022 00:04
@joshualitt joshualitt changed the title Migrate to JS static interop. Migrate package/flutter to JS static interop. Sep 10, 2022
@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Sep 10, 2022
@joshualitt joshualitt force-pushed the flutter-changes-for-wasm branch 3 times, most recently from 1bc3c0f to e97cf4d Compare September 12, 2022 23:04
@christopherfujino
Copy link
Contributor

if you rebase upstream to pick up 9ba3581 this diff will be a lot smaller

@joshualitt joshualitt force-pushed the flutter-changes-for-wasm branch from e97cf4d to bf8b98a Compare September 12, 2022 23:10
@joshualitt
Copy link
Contributor Author

@christopherfujino Thanks for the heads up! Much better.

@joshualitt joshualitt requested review from ditman and eyebrowsoffire and removed request for Piinks and christopherfujino September 13, 2022 00:04
@joshualitt
Copy link
Contributor Author

@ditman @eyebrowsoffire ptal. Thoughts? We can consolidate the shims into a single file, I just wasn't sure where it should live.

Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a 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?

@ditman ditman self-requested a review September 14, 2022 18:22
@joshualitt
Copy link
Contributor Author

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.

Copy link
Member

@ditman ditman left a 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 :)

@joshualitt joshualitt force-pushed the flutter-changes-for-wasm branch from bf8b98a to 5298d40 Compare September 16, 2022 17:20
@Hixie
Copy link
Contributor

Hixie commented Sep 16, 2022

test-exempt: code refactor with no semantic change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants