rrousselGit / riverpod

A reactive caching and data-binding framework. Riverpod makes working with asynchronous code a breeze.
https://riverpod.dev
MIT License
6.28k stars 955 forks source link

Riverpod Migrate Recommended Changing Import Path #840

Closed FXschwartz closed 3 years ago

FXschwartz commented 3 years ago

Describe the bug OS - Windows 11 Flutter Version - Flutter 2.5.2 Dart Version - Dart 2.14.3 Riverpod Version - hooks_riverpod: ^0.14.0+4 Riverpod CLI Version - riverpod_cli: ^1.0.0-dev.6

  1. Ran riverpod migrate in project root directory
  2. Displayed a recommended change to an import path that would break the import

To Reproduce I've only been able to reproduce this on my clients application

Expected behavior I'm not sure why it would recommend changing file import paths but I don't think it should.

image image

TimWhiting commented 3 years ago

That is very strange I'll look into it

FXschwartz commented 3 years ago

That is very strange I'll look into it

Thanks! There are a couple more cases I'll add screenshots for. One of which was the hooks_riverpod import.

TimWhiting commented 3 years ago

I feel like I'll need a minimally reproducible sample for this one. Are you using custom hooks that use providers? I feel like it is something related to this.

FXschwartz commented 3 years ago

@TimWhiting No I am not, in fact, I'm pretty close to replacing hooks completely and just using standard Riverpod. I don't believe there is any usage of it currently.

I can give you the entire file of one of the affected files? Would that help?

TimWhiting commented 3 years ago

Yes that would help. Looking at the current code this should not be happening, it should be impossible. You could try to use the migration tool from the master branch of this repo and see if it works.

FXschwartz commented 3 years ago

@TimWhiting I tried to separate out just this screen and it's dependencies but could not get the same bug to happen with it when running migrate although it happens everytime I run migrate on the production application. This is an odd one for sure. Here is the complete file.

import 'package:adaptive_dialog/adaptive_dialog.dart';
import 'package:flutter/material.dart';
import 'package:flutter_easyloading/flutter_easyloading.dart';
import 'package:flutter_form_builder/flutter_form_builder.dart';
import 'package:flutter_svg/flutter_svg.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:level_tap/models/user_data.dart';
import 'package:level_tap/providers/auth/auth_provider.dart';
import 'package:level_tap/providers/database/database_provider.dart';
import 'package:level_tap/providers/user/user_provider.dart';
import 'package:level_tap/screens/account/edit/widgets/company_input.dart';
import 'package:level_tap/screens/account/edit/widgets/email_input.dart';
import 'package:level_tap/screens/account/edit/widgets/first_name_input.dart';
import 'package:level_tap/screens/account/edit/widgets/last_name_input.dart';
import 'package:level_tap/screens/account/edit/widgets/phone_input.dart';
import 'package:level_tap/screens/account/widgets/user_profile_image.dart';
import 'package:level_tap/utilities/color_constants.dart';
import 'package:level_tap/widgets/text_shimmer.dart';

class AccountEditScreen extends StatelessWidget {
  AccountEditScreen({Key? key}) : super(key: key);
  final _formKey = GlobalKey<FormBuilderState>();

  Future<void> updateUserInfo(
    Map<String, dynamic> formData,
    BuildContext context,
    UserData user,
  ) async {
    final userAuth = context.read(userAuthRepositoryProvider);
    final database = context.read(databaseRepositoryProvider);
    EasyLoading.show(status: 'Updating info...');
    try {
      await userAuth.updateAuthUserInfo(
        formData['email'],
        formData['first_name'],
        formData['last_name'],
      );
      await database.updateUser(
        UserData(
          email: formData['email'],
          firstName: formData['first_name'],
          lastName: formData['last_name'],
          companyName: formData['company'],
          phone: formData['phone'],
        ),
        user.id,
      );
      EasyLoading.dismiss();
      context.refresh(userProvider);
      await showOkAlertDialog(
        context: context,
        title: 'Update Successful',
        message: 'Information successfully updated.',
      );
      Navigator.pop(context);
    } catch (err, stack) {
      print('updateUserInfo error: $err, stack: $stack');
      EasyLoading.dismiss();
      showOkAlertDialog(
        context: context,
        title: 'Failed To Update',
        message:
            'Information could not be updated at this time, please try again later.',
      );
    }
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        elevation: 0,
        toolbarHeight: MediaQuery.of(context).size.height * .08,
        centerTitle: true,
        title: SvgPicture.asset(
          'assets/logos/level-tap-logo.svg',
          width: 125.0,
        ),
        backgroundColor: kPrimaryColor,
        bottom: PreferredSize(
          child: Container(
            color: Colors.white,
            height: 2.0,
          ),
          preferredSize: const Size.fromHeight(2.0),
        ),
      ),
      body: SingleChildScrollView(
        child: Container(
          margin: const EdgeInsets.only(top: 50.0),
          child: FormBuilder(
            key: _formKey,
            autovalidateMode: AutovalidateMode.onUserInteraction,
            validationAutoFocus: false,
            child: Column(
              children: [
                const UserProfileImage(),
                const SizedBox(height: 10.0),
                FirstNameInput(formKey: _formKey),
                LastNameInput(formKey: _formKey),
                CompanyInput(formKey: _formKey),
                // const LocationInputs(),
                EmailInput(formKey: _formKey),
                PhoneInput(formKey: _formKey),
                const SizedBox(height: 20.0),
                Row(
                  mainAxisAlignment: MainAxisAlignment.spaceAround,
                  children: [
                    OutlinedButton(
                      onPressed: () {
                        Navigator.pop(context);
                      },
                      style: ButtonStyle(
                        side: MaterialStateProperty.all(
                          const BorderSide(color: Colors.white),
                        ),
                        backgroundColor: MaterialStateProperty.all(
                          kWarnButtonBackgroundColor,
                        ),
                        shape: MaterialStateProperty.all(
                          RoundedRectangleBorder(
                            borderRadius: BorderRadius.circular(
                              10.0,
                            ),
                          ),
                        ),
                      ),
                      child: const Text(
                        'BACK',
                        style: TextStyle(
                          color: kPrimaryColor,
                          fontWeight: FontWeight.bold,
                        ),
                      ),
                    ),
                    Consumer(builder: (context, watch, child) {
                      final userFutureProvider = watch(userProvider);
                      return userFutureProvider.when(data: (user) {
                        return OutlinedButton(
                          onPressed: () async {
                            if (_formKey.currentState!.saveAndValidate()) {
                              updateUserInfo(
                                  _formKey.currentState!.value, context, user);
                            } else {
                              showOkAlertDialog(
                                context: context,
                                title: 'Invalid Form Fields',
                                message:
                                    'Make sure all fields contains a valid value',
                              );
                            }
                          },
                          style: ButtonStyle(
                            backgroundColor: MaterialStateProperty.all(
                                kPrimaryButtonBackgroundColor),
                            shape: MaterialStateProperty.all(
                              RoundedRectangleBorder(
                                borderRadius: BorderRadius.circular(
                                  10.0,
                                ),
                              ),
                            ),
                          ),
                          child: const Text(
                            'UPDATE ACCOUNT',
                            style: TextStyle(
                              color: Colors.white,
                              fontWeight: FontWeight.bold,
                            ),
                          ),
                        );
                      }, loading: () {
                        return OutlinedButton(
                          onPressed: () async {},
                          style: ButtonStyle(
                            backgroundColor: MaterialStateProperty.all(
                                kButtonDisabledBackgroundColor),
                            shape: MaterialStateProperty.all(
                              RoundedRectangleBorder(
                                borderRadius: BorderRadius.circular(
                                  10.0,
                                ),
                              ),
                            ),
                          ),
                          child: const TextShimmer(
                            title: 'UPDATE ACCOUNT',
                            style: TextStyle(
                              color: Colors.white,
                              fontWeight: FontWeight.bold,
                            ),
                          ),
                        );
                      }, error: (err, stack) {
                        return const Text('Read Error');
                      });
                    }),
                  ],
                ),
              ],
            ),
          ),
        ),
      ),
    );
  }
}
TimWhiting commented 3 years ago

I have some ideas of why this might be happening, but this is a tricky one for sure.

FXschwartz commented 3 years ago

@TimWhiting Did some digging on this one and I've been able to condense the file down.

With this code, the same issue occurs except instead of recommending an import path edit, it recommends changing the updateUserInfo() function parameter context.

Not sure if this helps or not.

import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:level_tap/models/user_data.dart';
import 'package:level_tap/providers/auth/auth_provider.dart';
import 'package:level_tap/providers/database/database_provider.dart';
import 'package:level_tap/screens/account/edit/widgets/company_input.dart';

class AccountEditScreen extends StatelessWidget {
  AccountEditScreen({Key? key}) : super(key: key);
  // final _formKey = GlobalKey<FormBuilderState>();

  Future<void> updateUserInfo(
    Map<String, dynamic> formData,
    BuildContext context,
    UserData user,
  ) async {
    final userAuth = context.read(userAuthRepositoryProvider);
    final database = context.read(databaseRepositoryProvider);
  }

  @override
  Widget build(BuildContext context) {
    return const Scaffold();
  }
}

image

FXschwartz commented 3 years ago

One more weird example after I removed the unused company_input import

image

TimWhiting commented 3 years ago

@FXschwartz Sorry I haven't gotten to this yet.

I'm pretty sure this has something to do with the name of your providers and the hooks syntax.

Hooks start with use, and user starts with use.

Can you try renaming the userAuth provider to just plain auth provider, and trying again?

If this is the problem then it definitely needs to get corrected soon, because a lot of people will probably run into this.

You might have to changes the name of the UserAuthRepository class as well since some things look at the analyzed type.

I'll get to fixing this tonight if I can track down the issue.

FXschwartz commented 3 years ago

@TimWhiting No apologies needed at all! I'm sure you've got plenty of things on your plate, don't feel rushed.

I renamed userAuth -> auth and UserAuthRepository -> AuthRepository and the issue still persists.

import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:level_tap/models/user_data.dart';
import 'package:level_tap/providers/auth/auth_provider.dart';
import 'package:level_tap/providers/database/database_provider.dart';

class AccountEditScreen extends StatelessWidget {
  AccountEditScreen({Key? key}) : super(key: key);
  // final _formKey = GlobalKey<FormBuilderState>();

  Future<void> updateUserInfo(
    Map<String, dynamic> formData,
    BuildContext context,
    UserData user,
  ) async {
    final auth = context.read(authRepositoryProvider);
    final database = context.read(databaseRepositoryProvider);
  }

  @override
  Widget build(BuildContext context) {
    return const Scaffold();
  }
}

image

TimWhiting commented 3 years ago

@FXschwartz Can you check out this branch of my fork?

https://github.com/TimWhiting/river_pod/tree/fix_build_method

Then go into the packages/riverpod_cli folder and activate it using: dart pub global activate -s path .

Then try running riverpod migrate in your other project.

Not sure if it actually addresses this specific issue, because I haven't been able to replicate it, but it addresses some other issues I was seeing with migrating those files.

FXschwartz commented 3 years ago

@TimWhiting Sorry, still the same issue. Here's confirmation that I'm using your fork.

image

TimWhiting commented 3 years ago

Looks like you are on the master branch of my fork? Can you try switching branches to fix_build_method

FXschwartz commented 3 years ago

Oh my bad, but hey! That looks like it fixed it!

It is no longer recommending an import path change and is now correctly recommending an addition to the method parameters!

What do you think the issue was?

image

TimWhiting commented 3 years ago

When determining how to migrate functions that used the useProvider hook, I needed to keep track of the functions and their dependencies, so that I could thread through a WidgetRef to where it was needed. However, I was only identifying functions by names, and not by file or line. Making it be more explicit about unique functions with the same names fixed the problem.

You likely had multiple functions with the same name. Since I wasn't ensuring uniqueness of functions I was likely using the offset into the file from one function for migrating a different function in a different file. Obviously the file offsets will not line up and you get things inserted in weird places.

TimWhiting commented 3 years ago

Let me know if you see further issues. While the migration tool is not guaranteed to be flawless or know how to migrate everything, it should still be decent enough so that you don't have to work through a large migration by hand.