egg-core
egg-core copied to clipboard
add symbol type to loadToApp function property argument
Checklist
- [x]
npm testpasses - [x] documentation is changed or added
- [x] commit message follows commit guidelines
Affected core subsystem(s)
loader
Description of change
Add symbol type to the property argument of loadToApp function.
The js implementation supports this type of argument, but the typescript declaration does not support this, currently we can not pass symbol variable to be the property argument of loadToApp. As we use typescript heavily, we want to enable this type of argument.
Codecov Report
Merging #220 into master will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## master #220 +/- ##
=======================================
Coverage 99.80% 99.80%
=======================================
Files 19 19
Lines 1016 1016
=======================================
Hits 1014 1014
Misses 2 2
| Impacted Files | Coverage Δ | |
|---|---|---|
| lib/loader/egg_loader.js | 100.00% <ø> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update ca04a45...c2fe695. Read the comment docs.
when could we load to a symbol?
You can not do this in TS now, because the definition is not allowed, but you can do this in JS. This PR wants to fix the TS version problem.
my question is why will we call loadToApp to a symbol property?
I would suggest you ask the person who write the code in JS.
If the type of argument is supported by JS, then we should add a TS declaration for it, does this make sense?
sorry, could you show me the link for the js code that you just mentioned?
Please check:
https://github.com/eggjs/egg-core/blob/77e11f5fb38b3646fb20deb78bbc239b06bf8349/lib/loader/egg_loader.js#L379
And here: https://eggjs.org/zh-cn/advanced/framework.html#%E8%87%AA%E5%AE%9A%E4%B9%89-loader
The property is a key in a js map, and can be string, or Symbol, and you can see the official docs shows using a Symbol to be the key of a property.
Symbol.for('egg#loader') which is used for the framework, it's not the same thing with loadToApp.
this.app[property] doesn't mean it's suggested to use Symbol, it's just js, we don't have to modify the file to forbid Symbol.
and we don't find the scenario to loadToApp with some Symbol, so I think it should not add to typings.