Reduce use of globals in http and services classes

[krisgiesing]

Currently several services publish global objects. For example, asset_bundle.dart publishes rootBundle. http.dart publishes a long list of global functions like get and post.

These global objects and functions should be scoped to classes to avoid namespace issues.

[krisgiesing]

image_cache.dart publishes a global called imageCache.

[krisgiesing]

fetch.dart publishes fetch and fetchUrl.

[Hixie]

As long as it's only one, I'm not too worried. For example, imageCache is the entrypoint for the ImageCache logic, so it makes sense that it would be exposed at the top level.

http and fetch need a revamp, see #2889

[krisgiesing]

It's not a huge deal, but I do find it a little distracting to have a global object as a main entry point. On first read of client code these end up looking like local objects.

[Hixie]

You can always import the library with a prefix.

[sethladd]

These global objects and functions should be scoped to classes to avoid namespace issues.

Please consider using imports with prefixes. Also, if it helps, users can import libraries with show to be explicit where names come from. Scoping top-level function names inside of a class feels Java-y.

import "http.dart" show get; is an example of show in action.

[krisgiesing]

"On first read of client code" was referring to code I didn't write.

Anyway, it's not a huge deal. I logged this because @Hixie asked me to. :)

[Hixie]

I asked you to because you complained about it. :-)

Given that http/fetch are already covered by #2889, is the only issue here that imageCache.load starts with a lowercase "i" rather than an uppercase "I"? Or are http/fetch the real problem? Trying to figure out what the scope of this issue is.

[krisgiesing]

I just thought it was a strange practice to publish unscoped global objects and free-functions from the services classes. But if that's idiomatic Dart, then this should probably be closed.

FWIW, in the case of imageCache.load(), I would have expected something more like ImageCache.default.load() (or ImageCache.instance.load() if it's a singleton). For rootBundle.load(), I would have expected AssetBundle.root.load(). And so on.

[Hixie]

Ah, I see what you mean.

We did consider that. We ended up the way we have it because ImageCache.default.load() seems unnecessarily verbose compared to imageCache.load(), and we want the code to feel ergonomic. We could do ImageCache.load(), though, using a static. Not sure if that's better or worse.

[sethladd]

Do we intend to take any action here? If so, can we enumerate the actions in a task list in the issue description? If not, let's close.

FWIW, the http library was designed to be imported with a prefix. Putting that level of control into our users' hands has worked out. For very simple cases, you don't want to dirty your code with prefixes. For more complex code, the developer gets to pick a relevant prefix name. Not all libraries are designed to be imported with a possible prefix, though.

[krisgiesing]

Let's close.

[github-actions]

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.