fix: don't cache the intermediate locals for application #1146
Conversation
|
@JacksonTian, thanks for your PR! By analyzing the history of the files in this pull request, we identified @shaoshuai0102, @popomore and @fengmk2 to be potential reviewers. |
Codecov Report
@@ Coverage Diff @@
## master #1146 +/- ##
=========================================
- Coverage 99.7% 99.7% -0.01%
=========================================
Files 29 29
Lines 676 670 -6
=========================================
- Hits 674 668 -6
Misses 2 2
Continue to review full report at Codecov.
|
|
加个循环100万次的 app.locals = obj (每个 obj 1MB 大小)测试用例，确保不修改就好导致内存泄漏。。。 果然是不加测试用例提交 pr 的习惯改不掉。 |
lib/application.js
Outdated
| } | ||
| this[LOCALS_LIST].push(val); | ||
|
|
||
| assign(this[LOCALS], [ val ]); |
There was a problem hiding this comment.
不行的。如果确实就想放一个数组呢。
这个地方的设计说实话还是有点问题的，后加的会将前面的覆盖。但如果想撤销前面的，需要挨个 设置 {key: null}
There was a problem hiding this comment.
或者应该加严格的判断，app.locals = 必须是一个 Object 并不能是 Array。
There was a problem hiding this comment.
lib/application.js
Outdated
| @@ -73,21 +72,15 @@ class Application extends EggApplication { | |||
| * @see Context#locals | |||
| */ | |||
| get locals() { | |||
| if (!this[LOCALS]) { | |||
|
@JacksonTian 你不要改了，我修正了一个逻辑。等 ci 过了就合并。 |
| @@ -76,18 +75,15 @@ class Application extends EggApplication { | |||
| if (!this[LOCALS]) { | |||
Checklist
npm testpassesAffected core subsystem(s)
lib/application
Description of change
No more cache the intermediate locals for Application.