-
Couldn't load subscription status.
- Fork 37
feat: support pkgInfo.egg.require #45
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
Conversation
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.
+1
| if (eggInfo && eggInfo.require && Array.isArray(eggInfo.require)) { | ||
| execArgvObj.require = execArgvObj.require || []; | ||
| eggInfo.require.forEach(injectScript => { | ||
| execArgvObj.require.push(path.resolve(baseDir, injectScript)); |
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.
这里不应该 resolve 的,因为一般的用法是:
{
"name": "example",
"version": "1.0.0",
"egg": {
"require": [
"babel/register.js"
]
}
}加了 resolve 后,就得写成 ./node_modules/babel/register.js
这样和 egg-cluster 的 --require 表现不一致。
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.
其实 egg-bin 和 egg-scripts 这段逻辑,直接下沉到 egg-cluster 可能更适合。不过先修复这个再说
|
这个 PR 需要回滚,之前用户心智的 |
Checklist
npm testpassesAffected core subsystem(s)
Description of change
support:
though already support pkgInfo.eggScriptConfig.require , but it need to provide full path.
and more important is should act like egg-bin: https://github.com/eggjs/egg-bin/blob/8666e9eb9ce5016ac61af9f542b5518537a90a6b/lib/command.js#L84