D-James-GH / cached_query

Simple caching for flutter apps
MIT License
47 stars 10 forks source link

feat: add enabled flag to disable queries #38

Closed mauriciolcs closed 1 week ago

mauriciolcs commented 1 month ago

The purpose is to replicate the behavior of the enabled flag from Tanstack Query.

I needed to create the QueryConfig.copyWith function to keep the constructor constant, and I also needed to create a public setter for Query.config to update the enabled flag. I'm not sure if this is the best approach, so I'm open to suggestions.

D-James-GH commented 1 month ago

Hey, thank you for this. Do you have a use case example for this? The enable flag in react is required because you can't conditionally render a hook so the useQuery hook automatically calls the query function. Whereas, in flutter cached query you have to call result or listen to the stream in order to fetch the query, this can be done conditionally.

mauriciolcs commented 1 month ago

Got it, so maybe I just have to use my own builder to control when the .result is called?

For example, I have a menu that renders an item based on the user's role, and that same item depends on dynamic information that comes from the back-end, but I can only make the request if the user has the role.

I can't use QueryBuilder just on the item that needs the information and render conditionally because itemBuilder only accepts the PopupMenuItem widget

          QueryBuilder(
              query: getPersonalSupport(),
              builder: (context, state) {
                return Padding(
                  padding: const EdgeInsets.symmetric(horizontal: 3),
                  child: PopupMenuButton(
                    icon: Assets.icons.settings.svg(),
                    itemBuilder: (context) => [
                      if (authNotifier.user!.isPremium) ...[
                        PopupMenuItem(
                          enabled: state.data != null,
                          onTap: () {
                            openLink(state.data!.url);
                          },
                          child: const Row(
                            children: [
                              Icon(Icons.support, color: UiColors.primary),
                              SizedBox(width: 8),
                              Text('Support'),
                            ],
                          ),
                        ),
                        const PopupMenuDivider(),
                      ],
                      PopupMenuItem(
                        onTap: () => authNotifier.logout(),
                        child: const Row(
                          children: [
                            Icon(Icons.logout, color: Colors.red),
                            SizedBox(width: 8),
                            Text('Log out'),
                          ],
                        ),
                      ),
                    ],
                  ),
                );
              }),
D-James-GH commented 4 weeks ago

Thanks for confirming and sorry for the delay.

To you think it would be enough to add an enabled flag to the builders only? This would reduce the logic in the core package as you can just not call the result or stream from the core package. I haven't had a go at this yet to see whether it would be possible though.

D-James-GH commented 3 weeks ago

@mauriciolcs I've added an enable flag to the builder only on this branch: feat/builder-enable-flag. Would you mind having a look to see if this would work for your use case (I haven't added it to the infinite query yet).

Pros of the builder only:

Cons of builder only:

mauriciolcs commented 3 weeks ago

@mauriciolcs I've added an enable flag to the builder only on this branch: feat/builder-enable-flag. Would you mind having a look to see if this would work for your use case (I haven't added it to the infinite query yet).

Pros of the builder only:

  • not adding another way to enable/disable to the core.
  • Fully reactive, because the enable flag is part of the flutter framework changing the enable prop on the widget would be reflected in the UI.
    • In this PR, It looks like changing the config on the query wouldn't automatically update any listeners. It would probably work if you are setting enabled via setstate as this will likely create brand new query and widget and therefore re-subscribe. However, if the enabled flag is set outside the flutter framework (eg. in a repository) then that change would not be reflected in the UI. This could be a problem if you have a reference to a query in two places and one is then enabled. Correct me if I have missed something.

Cons of builder only:

  • enabled has to be set in multiple places if multiple query builders are being used on the same query.

Hi, thanks for your reply and I'm sorry, I forgot to answer you here 😅

Good point, there can indeed be this problem of reactivity. I hadn't thought about that.

I'm going to test this branch, but I think it will work in my case.

mauriciolcs commented 2 weeks ago

@D-James-GH the builder worked perfectly with the flag!

D-James-GH commented 1 week ago

Closing as this has been added now to the builder only as of version: cached_query_flutter: ^2.2.0