-
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
feat: loader support custom extension #156
Changes from 9 commits
7c202b2
030e489
49d967e
e710e8c
1073043
b9795ca
e718758
c85b114
88117fb
47635fc
5dc16bd
12307ec
8ce0beb
9b26274
c37fdb9
5363b4a
46dc810
50d2d6e
c51ae2d
8f2c5b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,4 +3,5 @@ coverage | |
.logs | ||
npm-debug.log | ||
.vscode | ||
.DS_Store | ||
.DS_Store | ||
yarn.lock |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ const deprecate = require('depd')('egg'); | |
const utils = require('../utils'); | ||
const FULLPATH = Symbol('EGG_LOADER_ITEM_FULLPATH'); | ||
const EXPORTS = Symbol('EGG_LOADER_ITEM_EXPORTS'); | ||
const MATCH = Symbol('EGG_LOADER_ITEM_MATCH'); | ||
|
||
const defaults = { | ||
directory: null, | ||
|
@@ -57,6 +58,22 @@ class FileLoader { | |
} | ||
} | ||
|
||
/** | ||
* default glob used to match files, creating by require.extensions. | ||
* @return {Array} match | ||
*/ | ||
get defaultMatch() { | ||
if (!this[MATCH]) { | ||
// cache the default match | ||
const extensions = Object.keys(require.extensions).map(ext => ext.substring(1)); | ||
this[MATCH] = [ '**/*.(' + extensions.join('|') + ')' ]; | ||
if (extensions.includes('ts')) { | ||
this[MATCH].push('!**/*.d.ts'); | ||
} | ||
} | ||
return this[MATCH]; | ||
} | ||
|
||
/** | ||
* attach items to target object. Mapping the directory to properties. | ||
* `app/controller/group/repository.js` => `target.group.repository` | ||
|
@@ -120,8 +137,12 @@ class FileLoader { | |
* @since 1.0.0 | ||
*/ | ||
parse() { | ||
let files = this.options.match || [ '**/*.js' ]; | ||
files = Array.isArray(files) ? files : [ files ]; | ||
let files = this.options.match; | ||
if (!files) { | ||
files = this.defaultMatch; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 如果这样的话,是不是干脆配置下默认的 match 就好了,不用 extensions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 大概想扩展吧,比如 jsx 啥的 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 那也是放到默认的 match 吧,现在这样子不就是根据 extensions 指定生成 match 么 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 我想了又想,也觉得不该用 require.extensions,毕竟是已经弃用了。。。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 可以的,改成 typescript There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 在 application 中提供一个 extensions 可供配置允许加载的后缀名会不会更好一些?方便拓展 jsx、mjs 等 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 要么就根据配置的后缀加载所有文件,然后根据是否扩展 require.extension 判断是否使用这个文件,比如 ts 通过 egg-cluster 开起 ts extension,这时就会加载 ts 了,而运行时如果只使用 js 就去除 扩展 extension There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 如果可以用 require.extensions,我就回滚到此前的 commit.(7c202b2) |
||
} else { | ||
files = Array.isArray(files) ? files : [ files ]; | ||
} | ||
|
||
let ignore = this.options.ignore; | ||
if (ignore) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
"devDependencies": { | ||
"autod": "^3.0.1", | ||
"coffee": "^4.1.0", | ||
"egg": "^2.5.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 实现在载 ts 就好,不要依赖 egg There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个是写 fixtures 的时候引入了 egg 的 types... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 如果去掉的话,就只能按照 js 的方式来写 ts 了... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 测试的时候自己加一个 ts 的 extension 好了,断言转后的是否符合预期。 这里只要测 loader 就好了,另外可以在 egg 里加个用例 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK,我改一下 |
||
"egg-bin": "^4.3.7", | ||
"egg-ci": "^1.8.0", | ||
"eslint": "^4.18.2", | ||
|
@@ -45,7 +46,9 @@ | |
"pedding": "^1.1.0", | ||
"rimraf": "^2.6.2", | ||
"spy": "^1.0.0", | ||
"supertest": "^3.0.0" | ||
"supertest": "^3.0.0", | ||
"ts-node": "^5.0.1", | ||
"typescript": "^2.7.2" | ||
}, | ||
"dependencies": { | ||
"co": "^4.6.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
'use strict'; | ||
|
||
const request = require('supertest'); | ||
const assert = require('assert'); | ||
const utils = require('./utils'); | ||
const tsNode = require('ts-node'); | ||
|
||
describe('test/egg-ts.test.js', () => { | ||
let app; | ||
|
||
it('should support ts-node', async () => { | ||
tsNode.register({ | ||
typeCheck: true, | ||
compilerOptions: { | ||
target: 'es2017', | ||
module: 'commonjs', | ||
moduleResolution: 'node', | ||
}, | ||
}); | ||
|
||
app = utils.createApp('egg-ts'); | ||
app.Helper = class Helper {}; | ||
app.loader.loadPlugin(); | ||
app.loader.loadConfig(); | ||
app.loader.loadApplicationExtend(); | ||
app.loader.loadAgentExtend(); | ||
app.loader.loadRequestExtend(); | ||
app.loader.loadResponseExtend(); | ||
app.loader.loadContextExtend(); | ||
app.loader.loadHelperExtend(); | ||
app.loader.loadService(); | ||
app.loader.loadController(); | ||
app.loader.loadRouter(); | ||
app.loader.loadPlugin(); | ||
app.loader.loadMiddleware(); | ||
app.loader.loadCustomApp(); | ||
app.loader.loadCustomAgent(); | ||
|
||
await request(app.callback()) | ||
.get('/') | ||
.expect(res => { | ||
assert(res.text.includes('from extend context')); | ||
assert(res.text.includes('from extend application')); | ||
assert(res.text.includes('from extend request')); | ||
assert(res.text.includes('from extend agent')); | ||
assert(res.text.includes('from extend helper')); | ||
assert(res.text.includes('from extend response')); | ||
assert(res.text.includes('from custom app')); | ||
assert(res.text.includes('from custom agent')); | ||
assert(res.text.includes('from plugins')); | ||
assert(res.text.includes('from config.default')); | ||
assert(res.text.includes('from middleware')); | ||
assert(res.text.includes('from service')); | ||
}) | ||
.expect(200); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { Application } from 'egg'; | ||
|
||
export default (app: Application) => { | ||
app.fromCustomAgent = 'from custom agent'; | ||
}; | ||
|
||
declare module 'egg' { | ||
interface Application { | ||
fromCustomAgent: string; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { Application } from 'egg'; | ||
|
||
export default (app: Application) => { | ||
app.fromCustomApp = 'from custom app'; | ||
}; | ||
|
||
declare module 'egg' { | ||
interface Application { | ||
Helper: any; | ||
fromCustomApp: string; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { Controller } from 'egg'; | ||
|
||
export default class HomeController extends Controller { | ||
public async index() { | ||
const { ctx, app } = this; | ||
const serviceText = ctx.service.test.getTest(); | ||
const helper = new ctx.app.Helper(); | ||
|
||
ctx.body = [ | ||
ctx.contextShow(), | ||
ctx.app.applicationShow(), | ||
ctx.request.requestShow(), | ||
ctx.response.responseShow(), | ||
ctx.app.agentShow(), | ||
helper.helperShow(), | ||
app.fromCustomApp, | ||
app.fromCustomAgent, | ||
app.config.test, | ||
app.config.testFromA, | ||
ctx.mid, | ||
serviceText | ||
].join(','); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import Home from './home'; | ||
|
||
declare module 'egg' { | ||
interface IController { | ||
home: Home; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
agentShow() { | ||
return 'from extend agent'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
applicationShow() { | ||
return 'from extend application'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
contextShow() { | ||
return 'from extend context'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
helperShow() { | ||
return 'from extend helper'; | ||
} | ||
} |
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.
如果用了 defaultMatch,就不需要再做 isArray 的判断了吧,因为就已经是 Array 了