-
Notifications
You must be signed in to change notification settings - Fork 708
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
Tighten guidelines around dynamic #6286
base: main
Are you sure you want to change the base?
Conversation
As the language and ecosystem has grown the target for static safety has moved up. Previously it was considered enough to be explicit about the places where `dynamic` is introduced, and then it is OK to silently do dynamic things with it, and write code that keeps references `dynamic` throughout. It is more common today to want to be explicit about places where `dynamic` is used, not just where it is introduced. We can see this in expanded use of stricter static checking and the `avoid_dynamic_calls` lint. Rephrase two guidelines to move towards being strict about using dynamic. - Recommend annotating with `Object?` instead of `dynamic` when inferences fails. Update the code sample to use `Object?` in the method arguments are return types for the JSON related API. Update the text to explain how `Object?` is treated differently and more strictly statically, which makes the unsafe operations visible in syntax. - Rephrase from using dynamic to "disable static checking" to using it to "invoke dynamic members". The wording for the rule already focused on using dynamic to "permit all operations". Remove the paragraph about retaining the `dynamic` type when working with APIs that use it, instead recommending to move away from `dynamic` at those boundaries and using `Object?` in most code.
Visit the preview URL for this PR (updated for commit e94a93a): https://dart-dev--pr6286-dynamic-guidelines-c9g9xz6n.web.app |
I'm not sure how we handle a change in recommendations that impacts a link header... Do we just allow old links to break? Edit: I'll fix existing local links if we decide to go forward with rewording these headers. |
Opening this now so folks can start considering it, but I expect we won't be able to organize all the discussion around it until after the holidays. cc @munificent what do you think? I think this change is a real shift in the ecosystem and I'm being descriptive here, but I do have a bias and I'd want to be prescriptive about this if were writing this doc with that intent. cc @lrhn who might have a preference to keep the older style which is more permissive. |
Since these headers usually last a long time and might be linked to from outside the site, if there's an similar or related header, I tend to add an empty anchor with the old fragment above that. For example: <a id="do-annotate-with-dynamic-instead-of-letting-inference-fail" aria-hidden="true"></a>
### DO annotate with `Object?` instead of letting inference fail |
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.
I'm fine with recommending that you don't write dynamic
.
Might want to say how you then opt back into dynamic behavior. I expect it's (… as dynamic).operation() as ExpectedType
.
to a more precise type before accessing members. | ||
Prefer using `Object?` over `dynamic` in code that is not invoking a member | ||
dynamically, even when working with existing APIs that use `dynamic`. | ||
For example, JSON objects have runtime type `Map<String, dynamic>`, which is an |
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.
I'd say "JSON objects traditionally have the static type Map<String, dynamic>
"
The runtime type doesn't matter, there is no meaningful difference between dynamic
and Object?
in a runtime type.
Also "JSON objects" is ambiguous. It can either mean any object created by jsonDecode
, which is typed as just dynamic
, or it can mean a Dart representation of a "JSON Object" (based on a JavaScript Object), which is indeed traditionally represented by a map with static type Map<String, dynamic>
.
The dart: convert
code doesn't mandate that you use Map<String, dynamic>
instead of Map<String, Object?>
when you match or cast to a Map
type, that's entirely user habit. It's this just saying that you should cast those to ... Object?...
before using them?
dynamically, even when working with existing APIs that use `dynamic`. | ||
For example, JSON objects have runtime type `Map<String, dynamic>`, which is an | ||
equivalent type to `Map<String, Object?>`. | ||
When using a value from one of these APIs, it's often a good idea to treat it as |
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.
Which APIs is this? (First use of "API" here, but the definite form suggests that it refers to something mentioned earlier.)
Is it something that expects a Map<String, dynamic>
, or which produces it?
Could the previous paragraph start with
"APIs which accept or produce JSON-object maps traditionally type those as Map<String, dynamic>
..."
As the language and ecosystem has grown the target for static safety has
moved up. Previously it was considered enough to be explicit about the
places where
dynamic
is introduced, and then it is OK to silently dodynamic things with it, and write code that keeps references
dynamic
throughout. It is more common today to want to be explicit about places
where
dynamic
is used, not just where it is introduced. We can seethis in expanded use of stricter static checking and the
avoid_dynamic_calls
lint.Rephrase two guidelines to move towards being strict about using
dynamic.
Object?
instead ofdynamic
wheninferences fails. Update the code sample to use
Object?
in themethod arguments are return types for the JSON related API. Update the
text to explain how
Object?
is treated differently and more strictlystatically, which makes the unsafe operations visible in syntax.
to "invoke dynamic members". The wording for the rule already focused
on using dynamic to "permit all operations". Remove the paragraph
about retaining the
dynamic
type when working with APIs that use it,instead recommending to move away from
dynamic
at those boundariesand using
Object?
in most code.