Hard to migrate from 6.x to 7.x

[kyptov]

Some methods just disapperead, no mention in guide, no deprecated. Unbind became async - here I gave up, too much rewriting.

Do you have plans to implement sync version of bind?

[notaphplover]

Hey @kyptov, I'm sorry to read that.

Some methods just disapperead, no mention in guide, no deprecated

Yeah, I understand the v7 migration is rough. I was talking about this topic with a contributor of the project. I wish I could have done an easier major, the technical challenge of migrating the core logic to fix some critical issues was a lot to take. Having said that. I though about adding deprecated methods, but keeping some v6 typings that should have never been exposed was a nightmare to maintain.

Which methods were removed and are not covered in the migration guide? Let me know and I'll have them a look to update the guide.

Do you have plans to implement sync version of bind?

Not really, but my plans are not set in stone and I am receiving similar feedback from other users, so I will consider an unbindSync even if it's not exactly what the nomenclature convention demands. Can I ask you to create a feature request please? Thanks in advance

[c100k]

Hey,

I'm referencing the same question I've just asked on the other repository, just for the sake of centralizing the discussion.

I'll add a +1 for rebind.

Generally speaking, I think we need to take a little step back here... I understand it's a lot of work but things are getting a little bit messy to be honest. There are 2 repositories, 2 documentation websites, undocumented breaking changes, very light changelogs, a very light migration guide... It's become really hard to find something.

Inversify has always been choice number 1 thanks to its stability and well defined API. To be honest, I had postponed the migration to v7 until today because of all the errors showing up when bumping, with no clear directions on how to address them in the migration guide.

Breaking things is fine as long as the major is bumped (which was the case here). But the changes also need to come with an exhaustive migration guide, otherwise it's just impossible to migrate. Removing functions is a really touchy operation on a library used by millions of people.

Again, you're really doing a great job on modernizing the library, there is no doubt about this. But slowing down a little bit and communicating better the changes could benefit everyone IMO.

[notaphplover]

Hey @c100k, thank you for your honest feedback.

Generally speaking, I think we need to take a little step back here... I understand it's a lot of work but things are getting a little bit messy to be honest. There are 2 repositories, 2 documentation websites, undocumented breaking changes, very light changelogs, a very light migration guide... It's become really hard to find something.

I see, I'm doing some efforts to make things easier. Container.unbindSync, Container.rebind and Container.rebindSync will be implemented. I'm working now on inversify/monorepo#558.

What are you missing in the migration guide? Your feedback is super important to improve the guide, if you can specify what are you struggling with (in addition to the absence of rebind), I'll be more than glald to update the guide.

Breaking things is fine as long as the major is bumped (which was the case here). But the changes also need to come with an exhaustive migration guide, otherwise it's just impossible to migrate. Removing functions is a really touchy operation on a library used by millions of people.

I totally agree with you. Just tell me what are you specificly missing and I'll take notes to update the guide ASAP.

Again, you're really doing a great job on modernizing the library, there is no doubt about this. But slowing down a little bit and communicating better the changes could benefit everyone IMO.

Thank you so much. I tried to announce new docs and the release of the major version. 5 alpha versions were released and I tried to collect as much feedback as possible, but I'm aware I can improve my soft skills. Any idea you have in mind to improve comunication with the users is more than welcome :)

[c100k]

Hello,

First, thanks a lot for reintroducting these methods. It will really help migrating to v7. Regarding the migration guide, I'll take an example :

In v6, container.resolve would automatically bind the resolved service to the container. In v7, this behavior has been removed. You can enable this behavior by passing the autobind option.

For instance, it was not clear to me that resolve() had disappeared when I read this behavior has been removed. I realized it only when tsc complained.

More generally, instead of sentences, introducing diffs for each change would really help. For example:

// v6
-container.resolve(MyClass)
// v7
+container.get(MyClass) // If no autobind or if autobind is defined in the container options
+container.get(MyClass, { autobind: true }) // If autobind only for this one

In addition to that, an exhaustive table with all the methods, imports, patterns that have been removed/renamed with their equivalent in v7 would really help. For example:

v6	v7	Comment
resolve(X)	get(X, { autobind: true})
rebind	???	No replacement because bla bla bla
load	await load
unload	await unload
unbind	await unbind
unbindAsync	unbind
interfaces.Provider	Provider

This gives a great overview of the changes and we all would have seen quickly if a method had been removed by error or just because you guys thought/considered it was not used by anyone :) (e.g. rebind).

That being said, I don't have all the answers but I really think it would help migrating.

[notaphplover]

Hey @c100k, I just deployed [email protected] introducing unbindSync, rebind and rebindSync. I think your suggestions are great, I'll try to take some time to apply them

[c100k]

Thanks @notaphplover ! With 7.3.0, all my tests turned green 🎉. I need to test in more details but so far so good.

The main things I had to apply:

• Rename autobindInjectable to autobind in container opts
• Change container.resolve(SomeClass) to container.get(SomeClass)
• Change container.rebind to await container.rebind when possible, otherwise container.rebindSync
• Use injectFromBase in one specific child class

[DenSpirit]

I have a codebase in v6 and it was very useful in testing to create a mock version of inversify container by taking a normal one and rebinding services I don't need.
One other example is rebinding a production DynamoDB client to a different one that targets local test endpoint. Unbinding other stuff that would otherwise attempt connecting to external sources if we missed it is also nice.

(This is of course for higher-level tests where entire request processing chain is tested. For unit tests on classes I use their constructors directly).

I tried to up to v7 and immediately noticed how rebind is gone and I need to make the beforeEach'es async to mimic original behavior.

Is there a better paradigm for testing with inversify available?

[notaphplover]

Hey @DenSpirit, [email protected] reintroduces rebind (async version) and rebindSync.

Is there a better paradigm for testing with inversify available?

It's a valid way to go. People either go that way or have a parent container with real services and build child containers overriding services with mocks. Both approaches are good ones.

[nicholas-ly]

In previous versions, I was able to use interfaces.Request to access the parentContext and resolve services conditionally based on certain configurations. However, with interfaces.Request replaced by BindingConstraints, it's unclear how I can now access the context, particularly the container and its bindings.

Unit tests using v6 to reproduce the scenario: https://stackblitz.com/edit/vitest-dev-vitest-ktiytq2s?file=test%2Fbasic.test.ts

This approach doesn't seem to work anymore in v7, and I can't find any documentation or migration guidance on how to access the context in the new version.

[nicholas-ly]

Hey @nicholas-ly,

In previous versions, I was able to use interfaces.Request to access the parentContext and resolve services conditionally based on certain configurations. However, with interfaces.Request replaced by BindingConstraints, it's unclear how I can now access the context, particularly the container and its bindings.

You can't. Custom binding constraint are part of the planning phase and no longer have access to resolution related API such as container.get. For your particaular case, it seems you just need to build your feature depending on your enabled-features related value:

import { Container } from 'inversify';

type FeatureFlag = 'beta-feature' | 'standard-feature';
type FeatureService = 'beta-service';

const container: Container = new Container();

container.bind<FeatureFlag>('enabled-features').toConstantValue('beta-feature');

container.bind<FeatureService>('feature-service').toResolvedValue(
  (enabledFeatures: FeatureFlag): FeatureService => {
    // Feel free to implement a builder or apply any other design pattern of your choice
    switch (enabledFeatures) {
      case 'beta-feature':
        return 'beta-service';
      default:
        throw new Error(`Unexpected enabled feature "${enabledFeatures}"`);
    }
  },
  ['enabled-features']
);

console.log(container.get('feature-service')); // Prints 'enabled-features'

This approach doesn't seem to work anymore in v7, and I can't find any documentation or migration guidance on how to access the context in the new version.

You're right. It's not clear in the migration guide. I'll update it.

How would you approach this if FeatureFlag is bound in its own independent container module?

[notaphplover]

Hey @kyptov,

It is obvious to us that the service must decide which client the repo will use. Of course, we could extend all our repos to *Transact versions, but we chose another approach. Each service injects a repo with @named(Transact) or @named(NonTransact). When the container resolves IDbClient, it looks at its parent name tag and provides either a Transact or Non-Transact client.

I guess you can simply use custom bindings for your clients. If your parent has @named(Transact), you can provide a DbTransactClient when parent has name Transact. Refer to docs for more information

[notaphplover]

Hey @nicholas-ly,

How would you approach this if FeatureFlag is bound in its own independent container module?

Same way, it should not matter since service resolution happens after container modules are loaded

[nicholas-ly]

Hey @nicholas-ly,

How would you approach this if FeatureFlag is bound in its own independent container module?

Same way, it should not matter since service resolution happens after container modules are loaded

Is it possible (or is there an alternative) for toResolvedValue(..) to not throw and simply not be bound to a service if the criteria (eg. the feature is not enabled) is not met?

[notaphplover]

Is it possible (or is there an alternative) for toResolvedValue(..) to not throw and simply not be bound to a service if the criteria (eg. the feature is not enabled) is not met?

I don't think so, your config is dynamic, it requires a service to be instantiated. The planning phase happens before. Having said that, I can't tell a real difference. Non bound services throw an error unless they are optional. If that's the case, you can recreate that behavior returning undefined instead of throwing.