Improving Solid widget and context.get<T>()

[manuel-plavsic]

At the moment, I think that DI is slightly unintuitive to use. There are some things that I think can be improved.

Using optional IDs for both providers and signals (in Solid widget)

Is there a technical reason why signals require an ID, while providers do not? In my experience:

• Most of the signals I insert with Solid are of a type for which there is only one signal. However, I still have to assign it an ID when using Solid and context.get<SignalBase<T>>.
• Some providers might have the same type, but it is not possible to use an ID to distinguish them.

Regarding this point, I would make IDs optional for both signals and providers.

So that I can write either:

signals: const {
  SignalIds.designLanguage: createDesignLanguageSignal,
  SignalIds.themeMode: createThemeModeSignal,
},

or

signals: const {
  createDesignLanguageSignal,
  createThemeModeSignal,
},

or

signals: const {
  createDesignLanguageSignal,
  createThemeModeSignal,
  // only here it makes sense to use IDs
  SignalIds.isPoweredOn:  createPoweredOnSignal, // type: Signal<bool>
  SignalIds.isLoggedIn:  createLoggedInSignal, // type: Signal<bool>
},

Of course this syntax doesn't make sense (mixing sets and maps isn't possible), but it is to give an idea. It would be neat to achieve a final solution where an ID does not always have to be passed.

context.get() is a bit unintuitive

I think it is a bit confusing to get a signal by passing an extra ID, otherwise an object that is not a signal will be injected.

I think that context.get could be deprecated in favour of context.getProvider, context.getSignal, context.getReadSignal and context.getResource (or maybe even better if we call them context.injectProvider, context.injectSignal, context.injectReadSignal and context.injectResource).

Merging providers and signals

Do you see a way so that we can merge the signals and providers fields of Solid, ending up only with providers, maybe? I don't think this is a necessity, but it might make things a bit simpler.

[nank1ro]

Using optional IDs for both providers and signals (in Solid widget)

This is wanted by design. You can have providers of the same type unless you specify them in different Solid widgets, e.g.:

class SomeClass {
  const SomeClass(this.text);
  final String text;
}

class Parent extends StatelessWidget {
  const Parent({super.key});

  @override
  Widget build(BuildContext context) {
    return Solid(
      providers: [
        SolidProvider<SomeClass>(
          create: () => const SomeClass('1'),
        ),
      ],
      child: const Child(),
    );
  }
}

class Child extends StatelessWidget {
  const Child({super.key});

  @override
  Widget build(BuildContext context) {
    return Solid(
      providers: [
        SolidProvider<SomeClass>(
          create: () => const SomeClass('2'),
        ),
      ],
      child: const InnerChild(),
    );
  }
}

class InnerChild extends StatelessWidget {
  const InnerChild({super.key});

  @override
  Widget build(BuildContext context) {
    return Text(context.get<SomeClass>().text);
  }
}

The following Text widget will render 2 because it finds the first SomeClass ancestor.
It's like saying that inner providers will override ancestor providers.
This is not a limit but a feature in my opinion.

---

signals: const {
  createDesignLanguageSignal,
  createThemeModeSignal,
},

This is not something that we're going to implement because it will become exactly like riverpod and we want to avoid that.
createDesignLanguageSignal must be a global signal and it's against the principles of this library.

There is no solution other than the one done by riverpod, but his solution requires global variables. So I think it's better to keep IDs.

context.get() is a bit unintuitive

I think it's pretty simple, and solidart_lint gives many lints when using context.get for accessing a provider or a signal.
I don't think that we should add all these methods to the library itself. If someone wants these additional methods (s)he can add the extensions into the project
To explain how to access stuff with Solid, there is just context.get, and not context.getProvider, context.getResource and so on.

Merging providers and signals

This will increase the complexity because I would have to add SolidSignal, SolidReadSignal, SolidResource and so on.
Now you just pass the createSignal|createComputed|createResource function and it works.

[manuel-plavsic]

Using optional IDs for both providers and signals (in Solid widget)

createThemeModeSignal and createDesignLanguageSignal are simple functions. I applied an eta reduction to () => createThemeModeSignal(). Its code is in the other discussion (I was sure I had linked it). Here is it again:

Signal<ThemeMode> createThemeModeSignal() {
  const key = 'theme_mode'; // shared preferences key
  final themeModeSignal = createSignal<ThemeMode>(
    () {
      final themeModeString = sharedPreferences.getString(key);
      if (themeModeString == null) {
        logger.v('No $key key found in shared preferences');
        sharedPreferences.setString(key, ThemeMode.system.name);
        return ThemeMode.system;
      } else {
        logger
          ..v('$key key found in shared preferences with value: $themeModeString')
          ..v('Valid ThemeMode strings: ${ThemeMode.values.map((ui) => ui.name)}');
        if (ThemeMode.values.map((ui) => ui.name).contains(themeModeString)) {
          // The value inside shared preferences is valid.
          final loadedUi =
              ThemeMode.values.singleWhere((ui) => ui.name == themeModeString);
          return loadedUi;
        } else {
          // The value inside shared preferences is not valid.
          const resetThemeMode = ThemeMode.system;
          sharedPreferences.setString(key, resetThemeMode.name);
          return resetThemeMode;
        }
      }
    }(),
  );
  createEffect((dispose) {
    final themeMode = themeModeSignal();
    sharedPreferences.setString(key, themeMode.name);
  });
  return themeModeSignal;
}

I am doing so to avoid ending up with long, difficult-to-read Solid widgets. This way, I just have:

Widget build(BuildContext context) => Solid(
        signals: {
          SignalIds.designLanguage: () => createDesignLanguageSignal(),
          SignalIds.themeMode: () => createThemeModeSignal(),
        },
        child:
        // ...

After eta reductions it looks like this (now the map can be a compile time constant):

Widget build(BuildContext context) => Solid(
        signals: const {
          SignalIds.designLanguage: createDesignLanguageSignal,
          SignalIds.themeMode: createThemeModeSignal,
        },
        child:
        // ...

Also, createDesignLanguageSignal could not be global signals since the signals map values need to be functions returning a signal. Thus, the ThemeMode and DesignLanguage signals are completely local.

Riverpod requires you to instantiate providers globally (e.g. final provider = Provider(...)). In our case, the instantiation happens in the solid widget. Thus, nothing like Riverpod.

Thus, i still think optional IDs would be very beneficial. Take for example (note: I know I can't mix map entries with set entries, but it is just to give an idea):

signals: {
  () => createDesignLanguageSignal(),
  () => createThemeModeSignal(),
  // only here it makes sense to use IDs
  SignalIds.isPoweredOn:  () => createPoweredOnSignal(), // type: Signal<bool>
  SignalIds.isLoggedIn: () =>  createLoggedInSignal(), // type: Signal<bool>
},

This way, one can simply call context.get<Signal<ThemeMode>>(), since there is not another SignalBase<ThemeMode> we need to distinguish it from. On the contrary, context.get<bool>() necessitates an ID so that isPoweredOn and isLoggedIn can be distinguished.

I think that the same should apply to providers as well. After all, DI is being done in either case, so it shouldn't really matter if the object extends SignalBase<T> or not.

context.get() is a bit unintuitive and Merging providers and signals

I see your points.

[nank1ro]

Sorry I misunderstood it.
The feasibility of this depends on detecting if T extends a SignalBase or not (meaning it's a Provider)

Will check this as soon as I have some time

[manuel-plavsic]

Then I propose to either:

• have just two functions: getSignal, whose generic type is a class extending SignalBase, and getProvider, whose generic type is S (with linting we can enforce that it must not extend SignalBase, or maybe this is even possible without linting?).
  Then getSignal can keep the optional params SignalIdentifier? id, while we can add one to getProvider (e.g. ProviderIdentifier? id). This way one can use IDs when really needed, i.e., in case of multiple signals/providers of the same type.
• or to keep using get, but we could change SignalIdentifier to DIIdentifier or SolidIdentifier (this might be a problem however, since detecting if T extends a SignalBase might not work).

[nank1ro]

Opened an initial PR here where I implemented the id for providers. You may check it out. In the issue I wrote what the next steps will be.

In particular I don't really like to have getProvider and getSignal but that's acceptable, and I don't like very much the variable name providerTypesOrIds for the Solid.value

[manuel-plavsic]

Wow, thanks for the PR! It looks very impressive. I am already thrilled to check it out tomorrow!