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.

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

Change status listener and page request listeners to final #7

Closed
wants to merge 1 commit into from
Closed

Change status listener and page request listeners to final #7

wants to merge 1 commit into from

Conversation

nicovogelaar
Copy link

This causes an error when using the TabBarView widget, which calls the dispose method when switching between tabs.

@EdsonBueno
Copy link
Owner

Do you mind sharing a toy project showcasing the error?

@nicovogelaar
Copy link
Author

This is the stack trace of the error:

[VERBOSE-2:ui_dart_state.cc(177)] Unhandled Exception: NoSuchMethodError: The getter 'iterator' was called on null.
Receiver: null
Tried calling: iterator
#0      Object.noSuchMethod (dart:core-patch/object_patch.dart:51:5)
#1      new List.from (dart:core-patch/array_patch.dart:57:19)
#2      PagingController.notifyStatusListeners (package:infinite_scroll_pagination/src/core/paging_controller.dart:134:28)
#3      PagingController.value= (package:infinite_scroll_pagination/src/core/paging_controller.dart:175:7)
#4      PagingController.appendPage (package:infinite_scroll_pagination/src/core/paging_controller.dart:84:5)
#5      PagingController.appendLastPage (package:infinite_scroll_pagination/src/core/paging_controller.dart:93:51)
#6      _FooListViewState._fetchPage (package:myproject/screens/foo_list.dart:129:27)
<asynchronous suspension>
#7      _FooListViewState.initState.<anonymous closure> (package:myproject/screens/foo_list.dart:86:59)
#8      PagingController.notifyPageRequestListeners.<<…>

I can provide a quick example of how the code looks like. Sorry, it is not a working example:

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:infinite_scroll_pagination/infinite_scroll_pagination.dart';

class FooListScreen extends StatefulWidget {
  @override
  State createState() => _FooListScreenState();
}

class _FooListScreenState extends State<FooListScreen> {
  @override
  Widget build(BuildContext context) {
    return DefaultTabController(
      length: 3,
      child: Scaffold(
        appBar: PreferredSize(
          preferredSize: Size.fromHeight(50.0),
          child: AppBar(
            backgroundColor: Colors.white,
            bottom: TabBar(
              labelColor: Colors.black,
              tabs: <Widget>[
                Tab(text: 'Status A'),
                Tab(text: 'Status B'),
                Tab(text: 'Status C'),
              ],
            ),
          ),
        ),
        body: Container(
          child: TabBarView(
            children: <Widget>[
              _FooListView(FooStatus.A),
              _FooListView(FooStatus.B),
              _FooListView(FooStatus.C),
            ],
          ),
        ),
      ),
    );
  }
}

class _FooListView extends StatefulWidget {
  final int status;
  _FooListView(this.status);
  @override
  State createState() => _FooListViewState();
}

class _FooListViewState extends State<_FooListView> {
  final PagingController<int, Foo> _pagingController =
      PagingController(firstPageKey: 0);

  @override
  initState() {
    _pagingController.addPageRequestListener((pageKey) => _fetchPage(pageKey));
    super.initState();
  }

  @override
  dispose() {
    _pagingController.dispose();
    super.dispose();
  }

  Future<void> _fetchPage(int pageKey) async {
    final pageSize = 10;

    final newItems = await RemoteAPI.getFoosByStatus(
      widget.status,
      limit: pageSize,
      offset: pageKey,
    );

    final isLastPage = newItems.length < pageSize;
    if (isLastPage) {
      _pagingController.appendLastPage(newItems);
    } else {
      final nextPageKey = pageKey + newItems.length;
      _pagingController.appendPage(newItems, nextPageKey);
    }
  }

  @override
  Widget build(BuildContext context) {
    return RefreshIndicator(
      onRefresh: () => Future.sync(() => _pagingController.refresh()),
      child: PagedListView<int, Foo>(
        pagingController: _pagingController,
        builderDelegate: PagedChildBuilderDelegate<Foo>(
          itemBuilder: (context, item, index) => _FooListItem(item),
          newPageProgressIndicatorBuilder: (context) => Center(
            child: CircularProgressIndicator(),
          ),
        ),
      ),
    );
  }
}

class _FooListItem extends StatelessWidget {
  final Foo _foo;

  _FooListItem(this._foo);

  @override
  Widget build(BuildContext context) {
    return Column(
      children: <Widget>[
        Card(
          child: ListTile(
            title: Padding(
              child: Row(
                mainAxisAlignment: MainAxisAlignment.spaceBetween,
                children: <Widget>[
                  Column(
                    crossAxisAlignment: CrossAxisAlignment.start,
                    children: <Widget>[
                      Text(
                        _foo.name,
                        textAlign: TextAlign.left,
                      ),
                    ],
                  ),
                  Icon(Icons.keyboard_arrow_right),
                ],
              ),
            ),
            onTap: () {},
          ),
        ),
      ],
    );
  }
}

@nicovogelaar
Copy link
Author

When I used AutomaticKeepAliveClientMixin for the ListView, the error was not occurring anymore.

@EdsonBueno
Copy link
Owner

EdsonBueno commented Oct 20, 2020

Alright!

Why is that happening? When you switch tabs without the AutomaticKeepAliveClientMixin, Flutter recreates your _FooListView state every time.
If you switch tabs while a _FooListView is fetching data from the API, that will cause you to call appendPage or appendLastPage on a dead controller when your API finally returns.

So, the problem is not the error being thrown, but the fact that your widget shouldn't execute code after being disposed.
This is a common issue people run into when working with Futures. Here's one quick way to fix it:

class _FooListViewState extends State<_FooListView> {
  final PagingController<int, Foo> _pagingController =
      PagingController(firstPageKey: 0);
  Object _activeCallbackIdentity;

  @override
  initState() {
    _pagingController.addPageRequestListener((pageKey) => _fetchPage(pageKey));
    super.initState();
  }

  @override
  dispose() {
    _pagingController.dispose();
    _activeCallbackIdentity = null;
    super.dispose();
  }

  Future<void> _fetchPage(int pageKey) async {
    final pageSize = 10;

    final callbackIdentity = Object();
    _activeCallbackIdentity = callbackIdentity;
    try {
        final newItems = await RemoteAPI.getFoosByStatus(
          widget.status,
          limit: pageSize,
          offset: pageKey,
        );

        if (callbackIdentity == _activeCallbackIdentity) {
            final isLastPage = newItems.length < pageSize;
            if (isLastPage) {
              _pagingController.appendLastPage(newItems);
            } else {
              final nextPageKey = pageKey + newItems.length;
              _pagingController.appendPage(newItems, nextPageKey);
            }
        }
    } catch (error) {
        if (callbackIdentity == _activeCallbackIdentity) {
            _pagingController.error = error;
        }
    }
  }

  @override
  Widget build(BuildContext context) {
    return RefreshIndicator(
      onRefresh: () => Future.sync(() => _pagingController.refresh()),
      child: PagedListView<int, Foo>(
        pagingController: _pagingController,
        builderDelegate: PagedChildBuilderDelegate<Foo>(
          itemBuilder: (context, item, index) => _FooListItem(item),
          newPageProgressIndicatorBuilder: (context) => Center(
            child: CircularProgressIndicator(),
          ),
        ),
      ),
    );
  }
}

This mechanism protects you against memory leaks and unexpected behavior whenever you’re working with Future calls. It’s nothing specific to pagination or infinite scrolling. FutureBuilder‘s implementation, for example, relies on it too.

Also, I took the liberty to add the error handling to your sample code.

You can reach me on our chat or my e-mail if you have any doubts!

Your PR quiets the error but doesn't solve the issue, that's why I'm closing it. Thank you so much for taking the time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants