4

I'm using inversify with inversify-express-utils.

I have a quite usual inversify/express setup.

  • Controller
    • Service
      • A module
        • B module

B module is a class that looks like this:

import { injectable } from 'inversify';
import SpellCorrector from 'spelling-corrector';

@injectable()
export class SpellCorrectorFactory {
  private corrector: any;

  constructor() {
    this.corrector = new SpellCorrector();
    this.corrector.loadDictionary();
  }

  public correct = (text: string): string => this.corrector.correct(text);
}

Now, the problem here is that as you can see in the constructor, I have this line of code:

this.corrector.loadDictionary()

This line takes over a second to execute. So basically it seems like the actual instance creation is happening when I @inject service B to service A So every time I make a request, the constructor of SpellCorrectorFactory is being executed, so the request takes over 1000ms instead of below 100ms.

How can I bind this class to inversify so that during binding the class is being instantiated and in the A module I have access to the instance which was created on app start, and not when I send a request to the express path?

Thanks in advance!

matewilk
  • 1,191
  • 1
  • 13
  • 27
  • 1
    I've never used it, but a cursory glance at their documentation indicates support for both singleton registrations and asynchronous factories. The latter is a rare feature in IOC containers. More to the point, why aren't you injecting `SpellCorrector`? As it is, your direct creation of that dependency nullifies the benefits of using such a framework – Aluan Haddad Aug 07 '20 at 04:06
  • I'm new to inversify, I though it would inject create instances on bind (sort of). You mean directly injecting the instance ? If I injected `SpellCorrector` I woulds still have to call the `loadDictionary()` method, which would mean I would have the same problem, wouldn't I ? I need the dictionary to be loaded on app start, not on every request. – matewilk Aug 07 '20 at 08:49
  • 1
    When you say "bind", do you mean setter injection? If so it's a bad practice. What I am talking about is constructor injection: `constructor(@inject(SpellCorrector) corrector: SpellCorrector) {}`. It's an interesting framework, but I'm new to it as well. How does `loadDictionary` work? It should return a `Promise`. If you are performing synchronous IO, you must stop. JavaScript applications are single-threaded. If it returns a `Promise`, you can register an asynchronous provider in inversify. – Aluan Haddad Aug 07 '20 at 09:10
  • Yes, I am injecting it as you describe it, but again, I need to call the method `this.corrector.loadDictionary` on application start, not in the express request path (where I'm injecting it). I NEED `loadDictionary` to be call on application START, when I do `npm run start`. Does it make sense ? – matewilk Aug 07 '20 at 09:26
  • 1
    Yes, it does make sense. However, since constructors are synchronous, and since you say that takes `loadDictionary` takes 1000+ms, blocking startup. I am asking you how `loadDictionary` works. Is it performing synchronous IO? – Aluan Haddad Aug 07 '20 at 09:31
  • I don't know what it does (it's a npm package) and it doesn't really matter, does it? I just want it to be called in a different stage of the application lifecycle. – matewilk Aug 07 '20 at 09:38
  • 1
    It matters a lot. If it is an async IO operation, then your `SpellCorrectorFactory` is in an initially invalid state when it is created, because of how you construct it. – Aluan Haddad Aug 07 '20 at 09:43
  • I checked, it doesn't load the dictionary with a request, it reads a file with `fs.readFileSync`. I still don't follow why would it matter. If I had the same problem with a different constructor, where method takes to long to execute or needs to be executed on app start, it wouldn't really matter what it does. The end result would be the same, I want it to be executed at the app start. It has nothing to do with the implementation. – matewilk Aug 07 '20 at 09:58
  • 1
    That it uses `fs.readFileSync` is terrible. JS is single threaded. That means that your application is 100% frozen, unable to do anything else while the file is loading. If it were properly written, it would use `fs.promises.readFile`. Then you could register it as a singleton, provided by an async factory, in inversify as documented [here](https://github.com/inversify/InversifyJS/blob/master/wiki/provider_injection.md#injecting-a-provider-asynchronous-factory), solving all. Seriously though, I cannot overstate how problematic sync IO is in a JS application, client or server, except CLI tools. – Aluan Haddad Aug 07 '20 at 10:04
  • I know the library is terrible, but the question is different. How to pass an INSTANCE to inversify. As simple as that. I really don't understand why we are talking about a library. It doesn't matter! And I don't mind my application to be frozen for 1000ms on the START. – matewilk Aug 07 '20 at 10:15
  • 1
    You brought up the delay yourself. How to pass an instance? Register it when you setup the your container: `container.bind(SpellCorrector).toFactory(() => {const c = new SpellCorrector(); c.loadDictionary(); return c; }).inSingletonScope()`. And remove the instantiation and dict loading from your `SpellCorrectorFactory`'s `constructor`. You might need to tweak that slightly, but the point is that this is how IOC works. You don't instantiate dependencies in the classes that depend on them. – Aluan Haddad Aug 07 '20 at 10:21
  • Yes, I brought up the delay myself to highlight where to problem is, meaning I want to move it from the route to the app start. It's a short term solution, I'll be looking to replace the library obviously. Thanks for the discussion and the solution. – matewilk Aug 07 '20 at 10:25
  • And with the above solution, I don't need the SpellCorrectorFactory class anymore – matewilk Aug 07 '20 at 10:26
  • 1
    No problem. Try it out. You should be able to make it work with `toFactory` but I haven't tested it naturally. When you upgrade to a better library, you can improve your startup time by changing `toFactory` to `toProvider`. – Aluan Haddad Aug 07 '20 at 10:27
  • 1
    I think I missed a nested function in there btw. It should be `container.bind(SpellCorrector).toFactory(() => () => { const c = new SpellCorrector(); c.loadDictionary(); return c; }).inSingletonScope()`. Anyway, it was interesting to look into this library. – Aluan Haddad Aug 07 '20 at 10:28
  • 1
    Yeah, it's really hard to figure out inversify, `inSingletonScope` is not a function of `toFactory`, I've tried yesterday for hours ... still no luck. So basically you end up with the same problem. As you cannot bind the factory to anything really ... – matewilk Aug 07 '20 at 10:47

1 Answers1

4

Ok, just in case someone looks at this page in search for an answer.

The solution is as simple as:

container
  .bind<SpellCorrectorFactory>(TYPES.SpellCorrector)
  .to(SpellCorrectorFactory)
  .inSingletonScope();

This calls the constructor of the SpellCorrectorFactory immediately and returns the instance. So whenever you inject the result of this binding you have direct access to the instance and its functions.

matewilk
  • 1,191
  • 1
  • 13
  • 27