Skip to content
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

fix(plugin loader): load plugin in npm flattened dependencies tree #173

Closed
wants to merge 1 commit into from

Conversation

BaffinLee
Copy link

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

try to find plugin in EGG_PATH/node_modules and EGG_PATH/../.

The problem I got

can't load buildin plugin like egg-onerror in test case of egg-script:

npm -v
# 5.6.0
node -v
# 8.11.3
yarn -v
# 1.7.0
git clone git@github.com:eggjs/egg-scripts.git
cd egg-scripts
npm install # yarn install
npm test

couldn't pass test case: test/egg-scripts.test.js -> start without daemon -> without baseDir, error:

➜  egg-scripts git:(master) ✗ npm test

> egg-scripts@2.6.0 test /Users/fake/code/egg-scripts
> npm run lint -- --fix && npm run pkgfiles && npm run test-local


> egg-scripts@2.6.0 lint /Users/fake/code/egg-scripts
> eslint . "--fix"


> egg-scripts@2.6.0 pkgfiles /Users/fake/code/egg-scripts
> egg-bin pkgfiles


> egg-scripts@2.6.0 test-local /Users/fake/code/egg-scripts
> egg-bin test



  test/egg-scripts.test.js
    ✓ show help (353ms)

  test/start.test.js
    start without daemon
      full path
        ✓ should start (10043ms)
cleanup: 1 to kill
cleanup master 2910
      relative path
        ✓ should start (10029ms)
cleanup: 1 to kill
cleanup master 2931
      without baseDir
[egg-scripts] Starting custom-framework application at /Users/fake/code/egg-scripts/test/fixtures/example
[egg-scripts] Run node /Users/fake/code/egg-scripts/lib/start-cluster {"workers":2,"title":"egg-server-example","framework":"/Users/fake/code/egg-scripts/test/fixtures/example/node_modules/custom-framework","baseDir":"/Users/fake/code/egg-scripts/test/fixtures/example"} --title=egg-server-example
2018-07-09 02:03:30,039 INFO 2953 [master] =================== custom-framework start =====================
2018-07-09 02:03:30,041 INFO 2953 [master] node version v8.11.3
2018-07-09 02:03:30,041 INFO 2953 [master] custom-framework version 1.0.0
2018-07-09 02:03:30,041 INFO 2953 [master] start with options:
{
  "framework": "/Users/fake/code/egg-scripts/test/fixtures/example/node_modules/custom-framework",
  "baseDir": "/Users/fake/code/egg-scripts/test/fixtures/example",
  "workers": 2,
  "plugins": null,
  "https": false,
  "key": "",
  "cert": "",
  "title": "egg-server-example"
}
2018-07-09 02:03:30,042 INFO 2953 [master] start with env: isProduction: true, EGG_SERVER_ENV: undefined, NODE_ENV: production
2018-07-09 02:03:30,053 INFO 2953 [master] agent_worker#1:2954 start with clusterPort:51371
/Users/fake/code/egg-scripts/node_modules/egg-core/lib/loader/mixin/plugin.js:354
    throw new Error(`Can not find plugin ${name} in "${lookupDirs.join(', ')}"`);
    ^

Error: Can not find plugin egg-onerror in "/Users/fake/code/egg-scripts/test/fixtures/example/node_modules, /Users/fake/code/egg-scripts/node_modules/egg/node_modules, /Users/fake/code/egg-scripts/test/fixtures/example/node_modules"
    at AgentWorkerLoader.getPluginPath (/Users/fake/code/egg-scripts/node_modules/egg-core/lib/loader/mixin/plugin.js:354:11)
    at AgentWorkerLoader.loadPlugin (/Users/fake/code/egg-scripts/node_modules/egg-core/lib/loader/mixin/plugin.js:108:26)
    at AgentWorkerLoader.loadConfig (/Users/fake/code/egg-scripts/node_modules/egg/lib/loader/agent_worker_loader.js:15:10)
    at new EggApplication (/Users/fake/code/egg-scripts/node_modules/egg/lib/egg.js:50:17)
    at new Agent (/Users/fake/code/egg-scripts/node_modules/egg/lib/agent.js:22:5)
    at Object.<anonymous> (/Users/fake/code/egg-scripts/node_modules/egg-cluster/lib/agent_worker.js:22:15)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
2018-07-09 02:03:30,442 ERROR 2953 nodejs.AgentWorkerDiedError: [master] agent_worker#1:2954 died (code: 1, signal: null)
    at Master.onAgentExit (/Users/fake/code/egg-scripts/node_modules/egg-cluster/lib/master.js:321:17)
    at emitOne (events.js:116:13)
    at Master.emit (events.js:211:7)
    at Messenger.sendToMaster (/Users/fake/code/egg-scripts/node_modules/egg-cluster/lib/utils/messenger.js:122:17)
    at Messenger.send (/Users/fake/code/egg-scripts/node_modules/egg-cluster/lib/utils/messenger.js:87:12)
    at ChildProcess.agentWorker.once (/Users/fake/code/egg-scripts/node_modules/egg-cluster/lib/master.js:220:22)
    at Object.onceWrapper (events.js:317:30)
    at emitTwo (events.js:126:13)
    at ChildProcess.emit (events.js:214:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:198:12)
name: 'AgentWorkerDiedError'
pid: 2953
hostname: MacBookPro.local

the plugin loader try to load egg-onerror plugin, in three path:

  • /Users/fake/code/egg-scripts/test/fixtures/example/node_modules
  • /Users/fake/code/egg-scripts/node_modules/egg/node_modules
  • /Users/fake/code/egg-scripts/test/fixtures/example/node_modules

but egg-onerror was installed into /Users/fake/code/egg-scripts/node_modules actually

Note:
  • use npminstall (cnpm) works fine, would pass every test cases in egg-scripts.
  • after apply my commit, works fine, would pass every test cases in egg-scripts.

@codecov
Copy link

codecov bot commented Jul 8, 2018

Codecov Report

Merging #173 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #173   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          18     18           
  Lines         958    959    +1     
=====================================
+ Hits          958    959    +1
Impacted Files Coverage Δ
lib/loader/mixin/plugin.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d910f6...d35716a. Read the comment docs.

@popomore
Copy link
Member

popomore commented Jul 9, 2018

Can you give me a repo to reproduce it.

@BaffinLee
Copy link
Author

@popomore just run test cases of egg-scripts and DO NOT use cnpm to install dependencies

git clone git@github.com:eggjs/egg-scripts.git
cd egg-scripts
npm install # yarn install
npm test

@atian25
Copy link
Member

atian25 commented Jun 21, 2022

#237

@atian25 atian25 closed this Jun 21, 2022
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.

None yet

3 participants