-
Notifications
You must be signed in to change notification settings - Fork 93
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
refactor: implement Loader instead of loading #4
Conversation
@popomore, thanks for your PR! By analyzing the annotation information on this pull request, we identified @gxcsoccer and @fengmk2 to be potential reviewers |
continue; | ||
} | ||
const properties = getProperties(filepath, this.options.lowercaseFirst); | ||
const exports = getExports(fullpath, this.options.initializer, this.options.call, this.options.inject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里的 exports 和 config 那里是不是换个变量名,毕竟和 commonjs 的 exports 全局变量同名了,万一不小心~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里应该不影响吧,作用域关系
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是不影响, 只是个人习惯不喜欢变量名覆盖, 一不小心就~
先这样吧.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
es6 完全没有这个问题。
@@ -285,6 +283,20 @@ class EggLoader { | |||
throw new Error('Can not get appname from package.json'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing name from package.json ?
这周身体出状况了,大家加油 |
+1 |
我把 loading 的用例移过来 |
添加了 loading 的用例 |
这个可以先合了 |
if (!/^[a-z][a-z0-9_-]*$/i.test(property)) { | ||
throw new Error(`${property} is not match 'a-z0-9_-' in ${filepath}`); | ||
} | ||
let result = property.replace(/[_-][a-z]/ig, function(s) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个应该是 let
+1 |
下个 pr 再改 const |
```bash egg start timeline: ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [289ms] - #0 Process Start ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [510ms] - #1 Application Start ▇ [2ms] - #2 Load Plugin ▇ [1ms] - #3 Load Config ▇ [0ms] - #4 Require(0) ~/eggjs/egg-core/test/fixtures/egg/config/config.default.js ▇ [0ms] - #5 Require(1) ~/eggjs/egg-core/test/fixtures/egg/config/config.unittest.js ▇ [0ms] - #6 Load extend/application.js ▇ [0ms] - #7 Require(2) ~/eggjs/egg-core/test/fixtures/egg/app/extend/application.js ▇ [1ms] - #8 Load extend/context.js ▇ [0ms] - #9 Load extend/request.js ▇ [0ms] - #10 Load extend/response.js ▇ [1ms] - #11 Load app.js ▇ [0ms] - #12 Require(3) ~/eggjs/egg-core/test/fixtures/egg/node_modules/session/app.js ▇ [0ms] - #13 Require(4) app.js ▇▇▇▇▇▇ [101ms] - #14 readyCallback in a ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [501ms] - #15 readyCallback in b ▇ [4ms] - #16 Load Middleware ▇ [4ms] - #17 Load "middlewares" to Application ▇ [2ms] - #18 Load Service ▇ [2ms] - #19 Load "service" to Context ▇ [0ms] - #20 Load Controller ▇ [0ms] - #21 Load "controller" to Application ▇ [0ms] - #22 Load Router ```
```bash egg start timeline: ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [289ms] - #0 Process Start ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [510ms] - #1 Application Start ▇ [2ms] - #2 Load Plugin ▇ [1ms] - #3 Load Config ▇ [0ms] - #4 Require(0) ~/eggjs/egg-core/test/fixtures/egg/config/config.default.js ▇ [0ms] - #5 Require(1) ~/eggjs/egg-core/test/fixtures/egg/config/config.unittest.js ▇ [0ms] - #6 Load extend/application.js ▇ [0ms] - #7 Require(2) ~/eggjs/egg-core/test/fixtures/egg/app/extend/application.js ▇ [1ms] - #8 Load extend/context.js ▇ [0ms] - #9 Load extend/request.js ▇ [0ms] - #10 Load extend/response.js ▇ [1ms] - #11 Load app.js ▇ [0ms] - #12 Require(3) ~/eggjs/egg-core/test/fixtures/egg/node_modules/session/app.js ▇ [0ms] - #13 Require(4) app.js ▇▇▇▇▇▇ [101ms] - #14 readyCallback in a ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [501ms] - #15 readyCallback in b ▇ [4ms] - #16 Load Middleware ▇ [4ms] - #17 Load "middlewares" to Application ▇ [2ms] - #18 Load Service ▇ [2ms] - #19 Load "service" to Context ▇ [0ms] - #20 Load Controller ▇ [0ms] - #21 Load "controller" to Application ▇ [0ms] - #22 Load Router ```
[skip ci] ## [5.3.0](v5.2.0...v5.3.0) (2023-01-13) ### Features * support show app.timing in timline string ([#260](#260)) ([5b7af12](5b7af12)), closes [#0](https://github.com/eggjs/egg-core/issues/0) [#1](#1) [#2](#2) [#3](#3) [#4](#4) [#5](#5) [#6](#6) [#7](#7) [#8](#8) [#9](#9) [#10](#10) [#11](#11) [#12](#12) [#13](#13) [#14](#14) [#15](#15) [#16](#16) [#17](#17) [#18](#18) [#19](#19) [#20](#20) [#21](#21) [#22](#22)
No description provided.