refactor API #5

popomore · 2016-08-02T09:54:28Z

做到灵活性，扩展性强。

API

loader 只提供原子粒度的 API，由框架来自行组织

Env

getServerEnv
getEggPaths
getLoadUnits
getAppname

Low Level API

loadFile
loadToApp
loadToContext
loadExtend

High Level API

和插件的关系

插件需要用到 loader 功能需要增加一个 loader 插件，比如 db 的功能

|- app
  `- db
    `- a.js

新增 egg-loader-db 插件

// app.js
module.exports = function(loader) {
  app.loader.loadToApp('app/db', 'db');
};

db 插件直接依赖 egg-loader-db 插件

load unit

每个加载单元的目录结构都是类似的，框架、插件、应用的路径都是称为 loadUnit。

|- app
  |- extend
  |- service
  |- controller
  |- middleware
  `- router.js
|- config
  |- config.js
  `- plugin.js
|- agent.js
`- app.js

LoaderOptions

directory: null,
target: null,
ignore: undefined,
lowercaseFirst: false,
initializer: null,
call: true,
override: false,
inject: undefined,

Action

eggjs/egg#19

This change is

mention-bot · 2016-08-02T09:54:29Z

@popomore, thanks for your PR! By analyzing the annotation information on this pull request, we identified @gxcsoccer and @fengmk2 to be potential reviewers

popomore · 2016-08-02T09:55:24Z

deps #4

Load order has changed from `app > framework > plugin > egg` to `app > framework(contain egg) > plugin` Remove options.eggPath, egg is one of the framework. Remove options.customEgg, every framework should specify `Symbol.for('egg#eggPath')`

changes - loadApplication > loadApplicationExtend - loadContext > loadContextExtend - loadRequest > loadRequestExtend - loadResponse > loadResponseExtend - loadHelper > loadHelperExtend

popomore · 2016-08-03T05:05:04Z

eggjs/egg#19 (comment) 把 loader 改完后再转成 core

loadUnit is a directory that can be loaded by EggLoader

popomore · 2016-08-03T13:24:35Z

发现在 loader service 的时候设置 call 为 false，但是 classLoader 里还会 call

~~去掉 classLoader 的调用，开启 loading 的 call~~

popomore · 2016-08-03T14:16:54Z

lib/egg_loader.js

+    this.serverEnv = this.getServerEnv();
+    debug('Loaded serverEnv %j', this.serverEnv);
+
+    this.appInfo = {


这个移到这里了，只生成一次

是否每个 config 都需要全新的 appinfo?现在这样如果改了会影响其他的。

不需要吧，如果需要自定义，覆盖对应的方法即可。

move proxy/service to mixin

popomore · 2016-08-03T16:01:50Z

原来还支持框架的 lib/plugins 目录加载插件

- framework's directory will change from lib/core to framework base - dont' lookup plugin from lib/plugins directory

fengmk2 · 2016-08-04T01:02:57Z

lib/egg_loader.js

+  /**
+   * Get all framework directories.
+   *
+   * You can extend Application of egg, the extrypoint is options.app,


fengmk2 · 2016-08-04T01:17:15Z

代码清晰多了。

popomore · 2016-08-04T07:54:42Z

Review status: 0 of 66 files reviewed at latest revision, 7 unresolved discussions.

lib/egg_loader.js, line 121 [r5] (raw file):

Previously, fengmk2 (fengmk2) wrote…

entrypoint

Done.

lib/mixin/config.js, line 90 [r5] (raw file):

Previously, fengmk2 (fengmk2) wrote…

这个可以去掉了。

Done.

Comments from Reviewable

popomore · 2016-08-04T08:30:53Z

Review status: 0 of 68 files reviewed at latest revision, 7 unresolved discussions.

lib/context_loader.js, line 26 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

classloader 是 ctx 级别的，应该会被释放吧

fengmk2 notifications@github.com于2016年8月4日周四上午8:58写道：
In lib/context_loader.js
#5 (comment):
constructor(options) {

assert(options.ctx, 'options.ctx is required');

const properties = options.properties;

this._cache = new Map();

this._ctx = options.ctx;
for (const property in properties) {
 this.defineProperty(property, properties[property]);
}

}
defineProperty(property, values) {

Object.defineProperty(this, property, {
 get() {
   if (!this._cache.has(property)) {
     this._cache.set(property, getInstance(values, this._ctx));
呃，_ctx 会一直被缓存着不释放？

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/eggjs/egg-loader/pull/5/files/98f7bcd5a5cae8e6cddbbf037796c850ca4db42d#r73448066,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWA1ZjXqt96z3m6t7ft4eV6TtQSrPC7ks5qcTkogaJpZM4Jabuw
.

Done.

lib/context_loader.js, line 61 [r5] (raw file):

Previously, fengmk2 (fengmk2) wrote…

~~ctx 改成 that？要不然看代码很容易误以为是 koa ctx~~

呃，还真是 ctx

Done.

lib/egg_loader.js, line 121 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

Done.

Done.

lib/egg_loader.js, line 283 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

不过升级 koa2 这边肯定要测过的

Done.

lib/mixin/config.js, line 85 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

那我不删了，这里都抛异常吧

Done.

lib/mixin/config.js, line 90 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

Done.

Done.

lib/mixin/middleware.js, line 38 [r5] (raw file):

Previously, fengmk2 (fengmk2) wrote…

想起来了。

Done.

Comments from Reviewable

popomore · 2016-08-04T08:53:09Z

reviewable 不会用

popomore · 2016-08-04T08:57:16Z

改好了

popomore · 2016-08-04T08:58:26Z

lib/context_loader.js, line 26 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

Done.

Done.

Comments from Reviewable

popomore · 2016-08-04T09:39:06Z

Reviewed 1 of 3 files at r1, 11 of 21 files at r2, 21 of 30 files at r3, 8 of 11 files at r4, 26 of 27 files at r5, 2 of 2 files at r6, 13 of 13 files at r7.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

lib/egg_loader.js, line 121 [r5] (raw file):

Previously, popomore (Haoliang Gao) wrote…

Done.

Done.

Comments from Reviewable

popomore · 2016-08-04T10:06:01Z

我知道为啥了 @fengmk2 也是 review 人

dead-horse · 2016-08-04T10:37:20Z

lib/egg_loader.js

+      serverEnv = process.env.EGG_SERVER_ENV;
+    }
+
+    if (!serverEnv) {


根据 NODE_ENV 做的逻辑判断最好是能够有个说明，不然感觉容易踩坑

这个放到使用文档说明吧，api 文档就不写了

and fix review

fengmk2 · 2016-08-05T06:10:31Z

+1

atian25 · 2016-08-05T15:48:44Z

README.md

+│   ├── service (optional)
+│   ├── middleware (optional)
+│   │   └── response_time.js
+│   └── view (optional)


```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)

popomore changed the title ~~refactor API~~ WIP: refactor API Aug 2, 2016

refactor: loadServerEnv -> getServerEnv

b7df00b

popomore force-pushed the api branch from 0e43e3a to 5932d68 Compare August 3, 2016 00:18

refactor: loadFrameworkPaths -> getEggPaths

3699562

Load order has changed from `app > framework > plugin > egg` to `app > framework(contain egg) > plugin` Remove options.eggPath, egg is one of the framework. Remove options.customEgg, every framework should specify `Symbol.for('egg#eggPath')`

popomore force-pushed the api branch from 5932d68 to 3699562 Compare August 3, 2016 03:21

refactor: export loadExtend

dffb7bc

changes - loadApplication > loadApplicationExtend - loadContext > loadContextExtend - loadRequest > loadRequestExtend - loadResponse > loadResponseExtend - loadHelper > loadHelperExtend

popomore added 10 commits August 3, 2016 13:13

test: add testcase for getAppname

16b0422

refactor: move loadFile and more testcase

f524dae

refactor: move load_custom_app/load_custom_agent

be89ac8

refactor: move middleware to loader

356c5a3

refactor: move controller to loader

7303490

refactor: rename loader to mixin

6734d41

refactor: move config to mixin

9a174d9

refactor: move plugin to mixin

8c2d1de

refactor: change loadDirs to getLoadUnits

3487004

loadUnit is a directory that can be loaded by EggLoader

refactor: rename base_loader to egg_loader

0b6a424

popomore assigned fengmk2, atian25 and gxcsoccer Aug 3, 2016

popomore reviewed Aug 3, 2016
View reviewed changes

feat: add loader.loadToContext, it can load files that bind ctx

ea88ab1

move proxy/service to mixin

popomore force-pushed the api branch from e30d33a to ea88ab1 Compare August 3, 2016 14:25

popomore added 2 commits August 4, 2016 00:15

refactor: don't load lib/core and lib/plugins

4b15dee

- framework's directory will change from lib/core to framework base - dont' lookup plugin from lib/plugins directory

refactor item_end to itesm_exports

98f7bcd

fengmk2 reviewed Aug 4, 2016
View reviewed changes

popomore force-pushed the api branch 2 times, most recently from 9804cfb to 306a71d Compare August 4, 2016 08:29

popomore changed the title ~~WIP: refactor API~~ refactor API Aug 4, 2016

dead-horse reviewed Aug 4, 2016
View reviewed changes

doc: update README

df70e19

and fix review

popomore force-pushed the api branch from 306a71d to df70e19 Compare August 4, 2016 16:34

fengmk2 merged commit 8a36509 into master Aug 5, 2016

fengmk2 deleted the api branch August 5, 2016 06:10

atian25 reviewed Aug 5, 2016
View reviewed changes

popomore restored the api branch October 17, 2016 09:42

popomore deleted the api branch December 19, 2016 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor API #5

refactor API #5

popomore commented Aug 2, 2016 •

edited

Loading

mention-bot commented Aug 2, 2016

popomore commented Aug 2, 2016

popomore commented Aug 3, 2016

popomore commented Aug 3, 2016 •

edited

Loading

popomore Aug 3, 2016

popomore Aug 4, 2016

fengmk2 Aug 5, 2016

popomore commented Aug 3, 2016

fengmk2 Aug 4, 2016

fengmk2 commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

dead-horse Aug 4, 2016

popomore Aug 4, 2016

popomore Aug 4, 2016

fengmk2 commented Aug 5, 2016

atian25 Aug 5, 2016

refactor API #5

refactor API #5

Conversation

popomore commented Aug 2, 2016 • edited Loading

API

和插件的关系

load unit

LoaderOptions

Action

mention-bot commented Aug 2, 2016

popomore commented Aug 2, 2016

popomore commented Aug 3, 2016

popomore commented Aug 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

popomore commented Aug 3, 2016

Choose a reason for hiding this comment

fengmk2 commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

popomore commented Aug 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengmk2 commented Aug 5, 2016

Choose a reason for hiding this comment

popomore commented Aug 2, 2016 •

edited

Loading

popomore commented Aug 3, 2016 •

edited

Loading