Potential issue with TileProvider's lifecycle

[vnmiso]

I wasn't sure if I should add an issue for this so I'll let a maintainer decide.

There's a fundamental issue with TileProvider: its lifetime is tied to a TileLayer's state's lifetime. How so? Because _TileLayerState.dispose() calls widget.tileProvider.dispose(). Notice that the tileProvider is held in the widget, aka TileLayer. In Flutter, this typically means that the widget doesn't own the passed in argument (unless explicitly stated by the documentation, but it isn't), so it shouldn't be responsible for managing its lifecycle.

This means that TileProviders are not reusable (also not mentioned in docs). If a TileProvider instance is ever passed into a TileLayer, it's going to be disposed whenever the TileLayer is removed from the tree.

Consider the following code:

class BrokenMap extends StatefulWidget {
  const BrokenMap({super.key});

  @override
  State<BrokenMap> createState() => _BrokenMapState();
}

class _BrokenMapState extends State<BrokenMap> {
  static final _tileProvider = NetworkTileProvider(
    headers: const {'My-Special-Header': 'That-Must-Be-Sent'},
  );
  var _isShowingStreets = true;

  @override
  Widget build(BuildContext context) {
    return FlutterMap(
      children: [
        if (_isShowingStreets)
          TileLayer(
            key: const Key('streets'),
            urlTemplate: 'https://streets.example.com/{z}/{x}/{y}.png',
            tileProvider: _tileProvider,
          )
        else
          TileLayer(
            key: const Key('satellite'),
            urlTemplate: 'https://satellite.example.com/{z}/{x}/{y}.png',
            tileProvider: _tileProvider,
          ),
        FilledButton(
          onPressed: () => setState(() => _isShowingStreets = !_isShowingStreets),
          child: const Text('Toggle layer'),
        ),
      ],
    );
  }
}

void main() => runApp(const MaterialApp(home: Scaffold(body: BrokenMap())));

When you press the button, tile loads will fail because NetworkTileProvider's internally constructed http client is closed in its dispose(). Since flutter_map v8, if you pass in your own http client you won't have this problem, but before v8 the externally passed in http clients were still closed so it still wouldn't work.

So you could say that I'm doing it wrong, and instead I should construct a fresh instance of TileProvider in every build, like so:

TileLayer(
  key: const Key('streets'),
  urlTemplate: 'https://streets.example.com/{z}/{x}/{y}.png',
  tileProvider: NetworkTileProvider(
    headers: const {'My-Special-Header': 'That-Must-Be-Sent'},
    // httpClient: _myClient,
  ),
)

But if you do it this way (and let's assume this is the only correct way), then you should be canonically expected to ALWAYS pass in your own http client. Why? Let's see.

If you don't pass in your own http client, a new one will be created on every build, which is already bad by itself, but you also lose the nice connection pool you had in the previous http client. And there's a bug in _TileLayerState: only the last TileProvider that it sees (through widget.tileProvider) is ever disposed, previous instances and their http clients will remain undisposed. (To solve this, intermediate tileProviders should be disposed in didUpdateWidget(), so I guess we do need a bug issue -- if TileLayer will remain the owner of the passed in TileProviders.)

So my conclusion is we probably need some docs around this stuff for TileProvider, and decide on who's responsible for a TileProvider's lifecycle. I'll try to make a PR after this is discussed andif I have time, but most likely I won't :(

P.S. I want to say a huge thanks to all the maintainers and contributors of flutter_map for this wonderful package, without which I wouldn't be able to do my work as well as with other Flutter mapping solutions.

[JaffaKetchup]

Hi @vnmiso, thanks for reaching out!

I do agree that we need to document the fact that tile providers aren't automatically reusable, and are attached to the TileLayer. In this case, I believe the best solution would be (since v8) to pass in a HTTP client if reusability is required.

It is potentially worth saying that swapping a TileLayer like that may be unnecessary in many use cases - you should be able to change most* of the properties, and it will update everything internally.

We do recommend in the docs that users should pass in their own HTTP client in the case that their TileProvider is constructed within the build method.

Regarding the bug in the _TileLayerState, that's a good catch! We don't generally expect users to need to change between tile providers within core FM, but once plugins are used, this can happen.

I'll try to come up with some concise way to represent the decision tree for the positioning of the TileProvider (and an HTTP client) depending on use case. FWIW, the documentation for FMTC does mention these factors, but I didn't copy it across to FM.

[JaffaKetchup]

I've updated the tile layer documentation, part of it to better describe how to optimize the tile provider/HTTP client.

[vnmiso]

Thank you, that touches on all the points! The "Choose a map source" section is great too.