Skip to content

feat: Support full generic-type registrations #400

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

Closed
wants to merge 1 commit into from
Closed

feat: Support full generic-type registrations #400

wants to merge 1 commit into from

Conversation

ps9310
Copy link

@ps9310 ps9310 commented Feb 6, 2025

Background

In Dart, generic type parameters are typically erased at runtime—two registrations like:

SomeGenericClass<Param1>
SomeGenericClass<Param2>

can be seen as the same Type, which causes collisions if you try to register both. By storing each registration under a string key (T.toString()) instead of Type, we preserve the full generic signature ("SomeGenericClass<Param1>" vs. "SomeGenericClass<Param2>") so that they remain distinct.

Key Changes

  1. String Keys in _Scope

    • Previously, _Scope stored registrations in something like:

      LinkedHashMap<Type, _TypeRegistration<T>>;
    • Now, it is:

      LinkedHashMap<String, _TypeRegistration<T>>;
    • Lookups and insertions rely on T.toString() (or (type ?? T).toString()) as the key.

  2. Lookups Use T.toString()

    • In places like _findFactoryByNameAndTypeOrNull, we do:

      final key = (type ?? T).toString();
      final typeRegistration = typeRegistrations[key];
    • Instead of using:

      typeRegistrations[type ?? T];
  3. Insertions Use T.toString()

    • When registering:

      typeRegistrations.putIfAbsent(T.toString(), () => _TypeRegistration<T>());
    • Thus, each new instantiation of a generic class is stored under its unique string representation.

  4. Minimal Structural Impact

    • We retain all original logic, variable names, and method signatures.
    • Only the lines that key or lookup typeRegistrations switched from Type to String.
    • Disposal, scoping, and reference-counting remain unchanged.

Benefits

  • Multiple Generic Registrations

    You can now do:

    getIt.registerLazySingleton<SomeGenericClass<A>>(() => SomeGenericClass(...));
    getIt.registerLazySingleton<SomeGenericClass<B>>(() => SomeGenericClass(...));

    without conflicts. They each go under a separate key:

    • "SomeGenericClass<A>"
    • "SomeGenericClass<B>"
  • No API Breakage

    • From a user’s perspective, the same calls to getIt<T>() or register*<T>() work as before.
    • All changes are purely internal to how get_it maintains its map of registrations.

- Switch internal maps from `Type` keys to `String` derived from `T.toString()`
- Allows distinct registration of generics like `SyncWssUseCase<A, B>` even when the base class is identical but type parameters differ
- Ensures lookups by full generic signature rather than erased `Type`
@ps9310
Copy link
Author

ps9310 commented Feb 6, 2025

Needs some improvements

@ps9310 ps9310 closed this Feb 6, 2025
@escamoteur
Copy link
Collaborator

escamoteur commented Feb 7, 2025 via email

@ps9310
Copy link
Author

ps9310 commented Feb 7, 2025

Of you use a string instead of the type, you lose the automatic type inference and it would make all previous code using get_it incompatible. Please discuss first before sending a PR Am 6. Feb. 2025, 23:48 +0100 schrieb Prashant Sawant @.>:

Needs some improvements — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.
>

@escamoteur, the automatic type inference works flawlessly. I’m not modifying the method signatures; instead, I’m adjusting the cache where we store the builders and instance information. There are no changes to how we use GetIt today—it continues to work without breaking changes. I validated this across multiple projects and existing tests, and everything functions correctly.

I’ve closed this PR due to many unnecessary changes. I’ll open a bug ticket to discuss this in detail.

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