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

docs: Address performance and stability concerns #12

Open
joebowbeer opened this issue Nov 28, 2021 · 3 comments
Open

docs: Address performance and stability concerns #12

joebowbeer opened this issue Nov 28, 2021 · 3 comments
Labels
documentation Improvements or additions to documentation

Comments

@joebowbeer
Copy link
Contributor

joebowbeer commented Nov 28, 2021

I suggest adding additional sections to the README or FAQ:

Performance concerns with AsyncLocalStorage?

This question sometimes arises because of early concerns with async_hooks performance.

See for example: iamolegga/nestjs-pino#322

Is there currently a concern? (I am not aware of any.)

Stability of AsyncLocalStorage?

According to Node documentation, AsyncLocalStorage is stable, even though the parent async_hooks is still experimental.

https://nodejs.org/api/async_context.html#class-asynclocalstorage

Could a more performant request-scoped provider implementation solve this problem?

I wouldn't be so sure that using AsyncLocalStorage atm would be better (performance-wise) than properly designed request-scoped providers. - from nestjs/nest#1407 (comment)

In addition to performance, another problem with request-scoped providers is the scope hierarchy implementation in NestJS:

Scope bubbles up the injection chain. A controller that depends on a request-scoped provider will, itself, be request-scoped.

See https://stackoverflow.com/a/59681868/901597

This provider does not have this restriction.

@Papooch Papooch added the documentation Improvements or additions to documentation label Nov 29, 2021
@Papooch
Copy link
Owner

Papooch commented Nov 29, 2021

I agree that this should be in the Readme - maybe under Security considerations? If you don't mind and have the time, you can suggest the edit in #11. I've not had much time lately, but I'm planning to add some git actions and revising the Readme by the end of the week.

@Papooch Papooch added this to the 2.0 milestone Feb 12, 2022
@Papooch Papooch removed this from the 2.0 milestone Jun 19, 2022
@micalevisk
Copy link
Contributor

I think it would be great to compare this lib with nestjs's durable providers feature in terms of performance gains.

@Papooch
Copy link
Owner

Papooch commented Dec 10, 2022

Yup, it would be nice to to have a benchmark, maybe someday I can make myself create one. However, it's important to note that AsyncLocalStorage and Request scoped providers don't have 100% feature overlap, so performance is not the key decision factor here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants