-
Notifications
You must be signed in to change notification settings - Fork 21
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
[CLOSE THIS][WIP] Completely and permanently replace continuation-local-storage with cls-hooked #10
Conversation
As soon as CLS module is loaded, the instrumentation/patching of async-listener is fired. This may cause stack overflows due to promise instrumentation. By loading CLS module lazily (only when used for real), we avoid this kind of problems in applications that are not using current-context at all.
Rename "per-request-context" middleware to shorter "per-request". Introduce "LoopBackContext.perRequest" as a shortcut for way too long "loopback-context/server/middlewar/per-request"
Recommend against using this module.
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
Can one of the admins verify this patch? |
|
as suggested by @raymondfeng in strongloop/loopback-context PR strongloop#1 comment #235931961
0759ad5
to
1047af4
Compare
I messed with rebase while trying to sync the fork with the upstream repository. I'm going to replace this with another PR with the same changes by me. |
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
Can one of the admins verify this patch? |
Messed up this PR, replaced with PR #11
Hello, this is a quick and (not so) dirty alternative to PR # 2.Rationale:Personally, I still like more the approach I used in PR # 2, but I just realized it needs further analysis, because I initially forgot to take into account issue async-listener#57.The quickest and most obvious way to address this is, of course, to get rid ofasync-listener
, which already was the long-term goal of my previous PR, anyway. Here it just turned into an immediate goal, that's all.One trivial way to reach this goal immediately is to completely replacecontinuation-local-storage
withcls-hooked
quitting any attempt to keep backward compatibility with the former, which is exactly what I did in this PR and is the difference with my previous PR. This should make the commitdf60f13
no longer necessary, so I reverted it.cls-hooked
, however (maybecontinuation-local-storage
too, I don't care), needs to be required before everything else, in order to not fail silently and catastrophically. So, I also gave proper and detailed instructions about how to do this in the README.mdAnyway, given the very short time I dedicated to this PR, I would consider this a work-in-progress for now.TODO:Bump version to 2.0.0-alpha.1 when it's more ready for alpha release.