Skip to content

Conversation

@atian25
Copy link
Member

@atian25 atian25 commented Sep 13, 2021

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

support:

{
  "name": "example",
  "egg": {
    "require": [
       "./inject.js"
    ]
  }
}

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

Copy link
Member

@hyj1991 hyj1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@atian25 atian25 merged commit fe179fd into master Sep 13, 2021
@atian25 atian25 deleted the require branch September 13, 2021 10:02
if (eggInfo && eggInfo.require && Array.isArray(eggInfo.require)) {
execArgvObj.require = execArgvObj.require || [];
eggInfo.require.forEach(injectScript => {
execArgvObj.require.push(path.resolve(baseDir, injectScript));
Copy link
Member Author

@atian25 atian25 Sep 14, 2021

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 表现不一致。

Copy link
Member Author

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 可能更适合。不过先修复这个再说

@atian25
Copy link
Member Author

atian25 commented Sep 14, 2021

这个 PR 需要回滚,之前用户心智的 pkgInfo.egg.require 是用于开发期的,如 babel/register 这些,不能在运行期加载的。

直接用 https://github.com/eggjs/egg-scripts/blob/fe179fda909cd7eb5b6497357202185a4ecf5ec6/lib/command.js#L68

hyj1991 added a commit that referenced this pull request Sep 14, 2021
atian25 pushed a commit that referenced this pull request Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants