rrousselGit / riverpod

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

CLI: Error when migrating large project #434

Closed AlexHartford closed 3 years ago

AlexHartford commented 3 years ago

The error:

[SEVERE] Suggestor.generatePatches() threw unexpectedly.
type 'DynamicTypeImpl' is not a subtype of type 'InterfaceType' in type cast

The code:

class ActiveSpeechDisplay extends HookWidget {
  const ActiveSpeechDisplay({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return IgnorePointer(
      child: AnimatedList(
        key: useProvider(speechList),
        shrinkWrap: true,
        reverse: true,
        physics: NeverScrollableScrollPhysics(),
        // Error caused by migrating this line (adding .notifier)
        initialItemCount: useProvider(SpeechItems.provider.notifier).visible.length,
        padding: const EdgeInsets.symmetric(horizontal: 100, vertical: 4),
        itemBuilder: (_, index, animation) => index < 4
            ? SpeechBubble(
                key: context.read(SpeechItems.provider.state)[index].key,
                animation: animation,
                index: index,
              )
            : Container(),
      ),
    );
  }
}

This then ends the migration process, and when I manually upgrade to riverpod 14.x, there is no error - the update the migration tool is making is correct. I could probably get through the migration by temporarily removing this part of my code, but wanted to raise this issue in case it's not a one-off bug.

Even after commenting out the line that causes the issue, even the provider declaration is throwing the same error.

The StateNotifier & Provider (post-migration):

class SpeechItems extends StateNotifier<List<SpeechBubbleModel>> {
  SpeechItems(this._read) : super([]);

  final Reader _read;

  // Error also caused by migrating this line (<SpeechItems> -> <SpeechItems, List<SpeechBubbleModel>>)
  static final provider =
      StateNotifierProvider.autoDispose<SpeechItems, List<SpeechBubbleModel>>((ref) => SpeechItems(ref.read));
  ...
}

I would be happy to provide the full StateNotifier if that could help but omitted that for now as I'm not sure it's relevant (and quite large).

TimWhiting commented 3 years ago

I can take a look later to see if I can reproduce, I doubt I'll need the full StateNotifier.

AlexHartford commented 3 years ago

Edited my original post to show that the provider declaration itself will also throw the same error when given the opportunity. I am also having this same error pop up throughout my project on my StateNotifiers. They are all following a similar pattern, so it makes sense that it would occur for all of them.

TimWhiting commented 3 years ago

Did you make sure that you had all of the version constraints at ^0.13.0? (you should not upgrade the version constraints before running the migration tool). The DynamicTypeImpl error would indicate that the analyzer wasn't able to statically analyze your code to determine all of the types (i.e. there were types that were unable to be inferred etc). I did find an issue related where: context.read(SpeechItems.provider.state)[index].key would not be converted to context.read(SpeechItems.provider)[index].key which I fixed via #441

AlexHartford commented 3 years ago

:facepalm: I had changed to 0.14.0+1, then reverted to 0.13.0+1 (doesn't exist) and pub didn't tell me I was wrong. So, after running it with 0.13.0, it works, although there are many pieces I have to manually migrate. When I have a minute to sit down and go through them I'll share if there's a pattern in what it didn't fix. Thanks for looking into it!

AlexHartford commented 3 years ago

I was able to get everything cleaned up and working. It missed removing .state and adding .notifier in several places, but the weirdest was the following case:

class BuiltTabs extends StateNotifier<List<bool>> {
  BuiltTabs(int numTabs) : super(List<bool>.filled(numTabs, false));

  static final provider =
      StateNotifierProvider.family<BuiltTabs, List<bool>, int>((_, numTabs) => BuiltTabs(numTabs));
}

Where the provider is read, the migration tool wrote:

final shouldBuildTab = context.read(BuiltTabs.provider<BuiltTabs, List<bool>>(navItems.length));

instead of the correct:

final shouldBuildTab = context.read(BuiltTabs.provide(navItems.length));

So it seems it didn't handle the StateNotifierFamily case correctly. My providers are all statically declared members of their respective StateNotifier as well (as can be seen above), so not sure if that could be impacting the tool as well.

TimWhiting commented 3 years ago

Hmm, I have a fix pending for the problems with transitioning from .state -> nothing when using a static provider, and the weird error when it adds type parameters to family accesses.

However, I'm unable to find any errors when it does not add .notifier. Any specific patterns there?

TimWhiting commented 3 years ago

Fix is now published as riverpod_cli: 0.0.2+2 for anyone who is looking at this issue.

AlexHartford commented 3 years ago

However, I'm unable to find any errors when it does not add .notifier. Any specific patterns there?

Not that I can see other than using static providers.

rrousselGit commented 3 years ago

Is this still an issue?

AlexHartford commented 3 years ago

It's not blocking me anymore, so I'll close it.