Skip to content

Add omit-type-checks flag for wasm builds#124341

Merged
auto-submit[bot] merged 10 commits intomasterfrom
i124177_omit_type_checks
Apr 11, 2023
Merged

Add omit-type-checks flag for wasm builds#124341
auto-submit[bot] merged 10 commits intomasterfrom
i124177_omit_type_checks

Conversation

@kevmoo
Copy link
Contributor

@kevmoo kevmoo commented Apr 6, 2023

Fixes #124177

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 6, 2023
@kevmoo kevmoo requested a review from eyebrowsoffire April 6, 2023 18:50
@kevmoo kevmoo added e: wasm Issues related to the wasm build of Flutter Web. platform-web Web applications specifically labels Apr 6, 2023
@kevmoo kevmoo marked this pull request as draft April 7, 2023 00:19
@flutter-dashboard
Copy link

flutter-dashboard bot commented Apr 7, 2023

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@kevmoo
Copy link
Contributor Author

kevmoo commented Apr 7, 2023

Trying a before/after:

Material 3 app (with some local tweaks)

normal: 6,940,575 bytes - 5,268,557 w/ wasm-opt

with --omit-type-checks

normal: 6,720,745 bytes - 5,045,495 w/ wasm-opt

CC @eyebrowsoffire @yjbanov

@kevmoo kevmoo marked this pull request as ready for review April 10, 2023 17:42
eyebrowsoffire
eyebrowsoffire requested changes Apr 10, 2023
View reviewed changes
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.

Could you write a unit test that actually ensures we properly add the flag? Maybe use

flutter/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart

Line 777 in 2379b18

test('Dart2WasmTarget invokes dart2wasm with dart defines', () => testbed.run(() async {
as a starting point for it?

@kevmoo kevmoo requested a review from eyebrowsoffire April 10, 2023 18:27
eyebrowsoffire
eyebrowsoffire approved these changes Apr 10, 2023
View reviewed changes
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.

LGTM!

@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 10, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 10, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Apr 10, 2023

auto label is removed for flutter/flutter, pr: 124341, due to - The status or check suite Windows framework_tests_widgets has failed. Please fix the issues identified (or deflake) before re-applying this label.

@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 10, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 10, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Apr 10, 2023

auto label is removed for flutter/flutter, pr: 124341, due to - The status or check suite Mac framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.

@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 10, 2023
@auto-submit auto-submit bot merged commit ecf6a66 into master Apr 11, 2023
@auto-submit auto-submit bot deleted the i124177_omit_type_checks branch April 11, 2023 00:38
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 11, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 11, 2023
@engine-flutter-autoroll
ecf6a66 Add omit-type-checks flag for wasm builds (flutter/flutter#12…
1eba3f5 
…4341)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 11, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 11, 2023
@engine-flutter-autoroll
ecf6a66 Add omit-type-checks flag for wasm builds (flutter/flutter#12…
96a307b 
…4341)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 11, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 11, 2023
@engine-flutter-autoroll
ecf6a66 Add omit-type-checks flag for wasm builds (flutter/flutter#12…
e7156eb 
…4341)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 12, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 12, 2023
@engine-flutter-autoroll
ecf6a66 Add omit-type-checks flag for wasm builds (flutter/flutter#12…
af4307a 
…4341)
exaby73 pushed a commit to exaby73/flutter_nevercode that referenced this pull request Apr 17, 2023
@kevmoo @exaby73
Add omit-type-checks flag for wasm builds (flutter#124341)
58806cf 
Add omit-type-checks flag for wasm builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eyebrowsoffire eyebrowsoffire eyebrowsoffire approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: wasm Issues related to the wasm build of Flutter Web. platform-web Web applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

web/wasm - wire --omit-type-checks through the build command

2 participants

@kevmoo @eyebrowsoffire