The Wayback Machine - https://web.archive.org/web/20200906062903/https://github.com/microsoft/tsyringe/issues/66
Skip to content
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

special handling of asyncs and promises #66

Open
xenoterracide opened this issue Dec 13, 2019 · 11 comments
Open

special handling of asyncs and promises #66

xenoterracide opened this issue Dec 13, 2019 · 11 comments

Comments

@xenoterracide
Copy link
Contributor

@xenoterracide xenoterracide commented Dec 13, 2019

so I have this code

export function context(): DependencyContainer {
  container.register(
    Beans.APOLLO_CONFIG,
    {
      useFactory: instanceCachingFactory<Promise<Config>>(async(c) => {
        return getApolloServerConfig();
      }),
    });
  return container;
}

getApolloServerConfig is an async function

now all the way down I have to do this

export function testContext(): DependencyContainer {
  const testContainer = context().createChildContainer();
  testContainer.register(
    TestBeans.APOLLO_SERVER,
    {
      useFactory: instanceCachingFactory<Promise<ApolloServer>>(async(c) => {
        const config = await c.resolve<Promise<Config>>(Beans.APOLLO_CONFIG);
        return new ApolloServer({
          ...config,
          engine: false,
        });
      }),
    });

  testContainer.register(
    TestBeans.APOLLO_TEST_CLIENT,
    {
      useFactory: instanceCachingFactory<Promise<ApolloServerTestClient>>(async(c) => {
        const server = await c.resolve<Promise<ApolloServer>>(TestBeans.APOLLO_SERVER);
        return createTestClient(server);
      }),
    });
  return testContainer;
}

including a final await in after my resolve.

what I'd really like to do (and maybe this isn't possible? is to return the promise on the first one, but have the following calls simply request the result of the first promise.

export function testContext(): DependencyContainer {
  const testContainer = context().createChildContainer();
  testContainer.register(
    TestBeans.APOLLO_SERVER,
    {
      useFactory: instanceCachingFactory<Promise<ApolloServer>>((c) => {
        const config = await c.resolve<Config>(Beans.APOLLO_CONFIG);
        return new ApolloServer({
          ...config,
          engine: false,
        });
      }),
    });

  testContainer.register(
    TestBeans.APOLLO_TEST_CLIENT,
    {
      useFactory: instanceCachingFactory<ApolloServerTestClient>((c) => {
        const server = await c.resolve<ApolloServer>(TestBeans.APOLLO_SERVER);
        return createTestClient(server);
      }),
    });
  return testContainer;
}

this may not be possible in any sort of sane rational way, I'm uncertain as I'm only recently coming to this from a Java/Spring world.

@mattbishop
Copy link

@mattbishop mattbishop commented Dec 17, 2019

Promises cache their values by default, so multiple calls to resolve() return the first resolution result. It's like a Java Future that way. https://www.ecma-international.org/ecma-262/6.0/#sec-promise-resolve-functions

@xenoterracide
Copy link
Contributor Author

@xenoterracide xenoterracide commented Dec 17, 2019

sure, but I'd rather not have to do container.resolve<Promise<Foo> when I could (in theory) just do container.resolve<Foo>

@Xapphire13
Copy link
Collaborator

@Xapphire13 Xapphire13 commented Dec 22, 2019

Dependency injection is by nature synchronous, so you would need to register the Foo instance after your first resolve in that case.

@xenoterracide
Copy link
Contributor Author

@xenoterracide xenoterracide commented Dec 23, 2019

I have come up with this as sort of a pattern, but I still question whether it might be possible to block on resolve, when registering a promise (yes I know DI is blocking by nature), basically a way of telling the container, this will be available, so wait for it...

  await createContentfulConnector(container.resolve(Beans.CONTENTFUL_CONNECTOR_CONFIG))
    .then((conn) => {
      container.register(Beans.CONTENTFUL_CONNECTOR, { useValue: conn });
    });
@Xapphire13
Copy link
Collaborator

@Xapphire13 Xapphire13 commented Dec 23, 2019

As far as I know, there is no way to make asynchronous things block in JavaScript (without using a native C/C++ module). The pattern you posted above is exactly what I was thinking, but you'd still need to make sure your program awaits the resolve/register before trying to resolve the actual instance (which would be the responsibility of the program, not the DI container).

I could see a case for adding a helper for the above pattern into tsyringe though (to guide usage to the pattern)

@xenoterracide
Copy link
Contributor Author

@xenoterracide xenoterracide commented Dec 27, 2019

I'd be ok with that, or maybe just some documentation. Though I can't help but wondering if maybe there's a way that a flag could be set internally for "building in progress" then also have that updated once the promise is resolved, in this way the resolve could be called as normal, but continue to block until ready.

another solution would be to inject proxies and do the blocking (if necessary) in there (is that possible?) though I would suggest some way of saying proxy mode is acceptable.

@Xapphire13
Copy link
Collaborator

@Xapphire13 Xapphire13 commented Dec 31, 2019

I think the best way to handle this right now would be to update the documentation for this case. Even if the container internally tracked whether or not "building is in progress", there would still need to be some external await ... since we can't force JS to block. Because of that, I feel the solution should live entirely outside of the container (via the helper function you suggested for example)

@xenoterracide
Copy link
Contributor Author

@xenoterracide xenoterracide commented Jan 3, 2020

so, I wrote this...

export function factory(explicit?: symbol, { singleton = true, container = rootContainer() } = {}) {
  return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
    const token = !!explicit ? explicit : Symbol.for(propertyKey);
    const factory = (c: DependencyContainer) => {
      const obj = descriptor.value.apply(target, [c]);
      if (obj instanceof Promise) {
        const handler = {
          get: (target: Promise<any>, prop: string, receiver: any) => {
            return target.then((o) => {
              return o[prop].apply(o);
            });
          },
        };
        return new Proxy(obj, handler);
      }
      return obj;
    };
    const finalFactory = singleton ? instanceCachingFactory(factory) : factory;
    container.register(token, { useFactory: finalFactory });
  };

would something like proxyForPromiseFactory(...) be a good idea to fulfill this? (this is a first attempt, if you see something broken please let me know)

@xenoterracide
Copy link
Contributor Author

@xenoterracide xenoterracide commented Jan 14, 2020

looked into the proxy thing more... it doesn't work, the more I think about this, I'm not sure it's a good idea, in our case it's me trying to support an existing anti-pattern.

@Xapphire13
Copy link
Collaborator

@Xapphire13 Xapphire13 commented Jan 17, 2020

Yeah, I still think the best fix for this is to document how to handle async resolutions

@luiz290788
Copy link

@luiz290788 luiz290788 commented Jun 28, 2020

I was reading this issue and thinking if there is a better way of doing this. Can we specify on the arguments of the factory the dependencies to build the new bean and then the framework can orchestrate to call the factory function just when all their dependencies are resolved?

Something like this:

  testContainer.register(
    TestBeans.APOLLO_TEST_CLIENT,
    {
      useFactory: instanceCachingFactory<Promise<ApolloServerTestClient>>(TestBeans.APOLLO_SERVER, (c, server: ApolloServer)
        => createTestClient(server)),
    });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.