search

Milad-Akarie / injectable

Code Generator for get_it
MIT License
557 stars 143 forks source link

Feature: Stabilise the service name aliases #440

Closed Adam-Langley closed 2 months ago

Adam-Langley commented 6 months ago

Requested by @saschaernst here: #438

Code generation has recently been fixed, to ensure deterministic output of the get_it.config.dart file (same input, same output).

An issue remains (a usability issue - hence this being a feature request), that the aliases generated for service names (i.e _i19.FooServiceClass, _i20.BarServiceClass) are based on something other than the DependencyConfig identity (assumption that it uses something related to the order of the services in an internal data structure).

The effect is that a change to the order of these services can cause the aliases of unrelated services to change throughout the entire file. One small change to a single service registration can result in hundreds of alias changes throughout the file - unnecessary noise.

Due to #438, the DendencyConfig class now has an integer field which represents its globally unique identity - identityHash.

This feature request is to change the code generator to utilise that hash as the integer component of the alias name.

saschaernst commented 5 months ago

May I humbly ask if anybody plans on taking on this task?

Adam-Langley commented 5 months ago

Hi @saschaernst I'm just replying here as I instigated the other thread... I don't have any plans to work on this item myself. If I get some spare time (which I don't expect for at least the next 2 months) then I would look at it. Of course, that's no reflection on the value of this item - I think it would be a great usability enhancement. Happy to contribute to discussions if someone else wants to run with this.

saschaernst commented 5 months ago

Thanks @Adam-Langley, as the hashes are already implemented, could you estimate the amount of work and complexity to use them as aliases?

Adam-Langley commented 5 months ago

Hi @saschaernst - you've inadvertently touched on exactly why I wouldn't look at this right now :) I don't really have a confident feel for the code, so I could only muster a gut feel - probably about 1 hour including tests for @Milad-Akarie - at least 4 hours for me.

The change I did make on the other PR was an order of magnitude simpler - just a basic patch, vs writing new structural classes as @Milad-Akarie has suggested. I had an initial crack at it - but I just couldn't find the code which was responsible for generating the aliases. Admittedly, I was working blind (I havent set up a debugging environment for this library). Doing so would make it much more obvious - but I drew the line there (billable work and all).

sorokinDev commented 5 months ago

I had an initial crack at it - but I just couldn't find the code which was responsible for generating the aliases. Admittedly, I was working blind (I havent set up a debugging environment for this library). Doing so would make it much more obvious - but I drew the line there (billable work and all).

Hi @Adam-Langley! I recently did a change for stable aliases in injectable's fork inside our company, as we were struggling with a lot of conflicts in PRs. Related code is located here: https://github.com/Milad-Akarie/injectable/blob/master/injectable_generator/lib/generators/injectable_config_generator.dart#L171. Allocator.simplePrefixing is the one creating _i1, _i2.... I implemented own Allocator and replaced it here. I used hash of import url, like so: hash('package:foo/bar.dart'). And it works fine!) If you don't mind, I can create a PR within a couple of weeks.

Adam-Langley commented 5 months ago

Hi @sorokinDev - well done - I'll be keen to see it when you're ready.

ueman commented 4 months ago

@sorokinDev Can you share your code?

jamontes79 commented 3 months ago

@Adam-Langley any news about the open PR https://github.com/Milad-Akarie/injectable/pull/465 ?

Adam-Langley commented 3 months ago

Hi, I haven't had a look at it - been flat out with production work. Unfortunately I'm not a project owner in this, so I could not approve it anyway. I see you've ping'd Milad - he's super busy too but I'm sure he'll get to it. In the mean time, what I do is use a dependency override in my pubspec so PR's don't hold me back. In fact, I'm totally in love with dart+github in that respect - working off a fork is so frictionless, even with a CI server. Not like the days of binary dependencies (ahem, .net)