Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update documentation when we make --delete-tostring-package-uri defaulted for certain modes #52759

Closed
dnfield opened this issue Mar 17, 2020 · 6 comments · Fixed by #144763
Closed

Update documentation when we make --delete-tostring-package-uri defaulted for certain modes #52759

dnfield opened this issue Mar 17, 2020 · 6 comments · Fixed by #144763
Labels
d: api docs Issues with https://api.flutter.dev/ engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@dnfield
Copy link
Contributor

dnfield commented Mar 17, 2020

flutter/engine#17068 will add a new option to the frontend compiler to let us remove toString options, along with an annotation to mark a method as keep even if we applied the flag.

We should update the docs on the annotation if and when we make this option on by default for, e.g. release or profile modes, to reflect when it's on.

/cc @zanderso @jonahwilliams
@dnfield dnfield added engine flutter/engine repository. See also e: labels. d: api docs Issues with https://api.flutter.dev/ documentation labels Mar 17, 2020
@dnfield dnfield added this to the Goals milestone Mar 17, 2020
@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label May 29, 2020
@Hixie Hixie removed this from the None. milestone Aug 17, 2020
@pedromassangocode
Copy link

Can you confirm where to verify if the docs were updated already or not?

@pedromassangocode pedromassangocode added waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds in triage Presently being triaged by the triage team labels Dec 2, 2020
@dnfield
Copy link
Contributor Author

dnfield commented Dec 2, 2020

I'm not sure we ever turned this on by default. @jonahwilliams do you know?

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Dec 2, 2020
@dnfield
Copy link
Contributor Author

dnfield commented Dec 2, 2020

Relevant docs to update are here: https://github.com/flutter/engine/blob/d85cb10707a29dae9a437a1cf6bfa2e28860d3f7/lib/ui/annotations.dart#L11

If we didn't turn this on by default, then we can probably close this. This option is used by some customers, but AFAICT they have to pass it in themselves via some special flags when building the app.

@jonahwilliams
Copy link
Member

We don't have this on by default. Before we do this, we'd need to provide it as an opt-in and gather feedback - otherwise users might be surprised.

@dnfield
Copy link
Contributor Author

dnfield commented Dec 2, 2020

It's currently possible to opt-in with the --extra-front-end-options flag I believe. It's just cumbersome to use. We have some customers internally at Google using it though.

I'm inclined to leave it as is for now, unless we get more demand for it.

@pedromassangocode pedromassangocode added passed first triage and removed in triage Presently being triaged by the triage team labels Dec 3, 2020
@flutter-triage-bot flutter-triage-bot bot added team-engine Owned by Engine team triaged-engine Triaged by Engine team labels Jul 8, 2023
mkustermann added a commit to mkustermann/engine that referenced this issue Mar 7, 2024
@mkustermann
Make @keepToString annotation work for web and non-web
1ee351d 
* Update documentation for the annotation
* Add `@keepToString` annotation to web's `Locale` class (similar to
  non-webs `Locale` class)

Issue flutter/flutter#52759
@mkustermann mkustermann mentioned this issue Mar 7, 2024
Merged
mkustermann added a commit to flutter/engine that referenced this issue Mar 7, 2024
@mkustermann
Make @keepToString annotation work for web and non-web (#51251)
40a4279 
* Update documentation for the annotation
* Add `@keepToString` annotation to web's `Locale` class (similar to
non-webs `Locale` class)

Issue flutter/flutter#52759
@mkustermann mkustermann mentioned this issue Mar 7, 2024
Merged
mkustermann added a commit that referenced this issue Mar 8, 2024
@mkustermann
Remove toString() overrides in dart:ui/package:flutter in profi…
7c89ec8 
…le/release mode on wasm/vm targets (#144763)

The flutter engine & framework can opt out of this optimization for
individual classes / class hierarchies via:

  * `@pragma(flutter:keep-to-string)`
  * `@pragma(flutter:keep-to-string-in-subtypes)`

Or by using the convenience constant `@keepToString` from `dart:ui`.

=> This aligns the build process more with g3 (which already does this)

Closes #52759
@github-actions GitHub Actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
d: api docs Issues with https://api.flutter.dev/ engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
None yet
Milestone
No milestone
5 participants
@Hixie @kf6gpe @dnfield @jonahwilliams @pedromassangocode