jackfranklin / gulp-load-plugins

Automatically load in gulp plugins
https://github.com/jackfranklin/gulp-load-plugins
MIT License
757 stars 55 forks source link

Fixed bug where config option wasn't that useful in child dirs. #57

Closed callumacrae closed 9 years ago

callumacrae commented 9 years ago

Fixes #56. I feel this probably needs a test, but I'm not sure how to test it without adding a test dir and extra files. It works in my case, and none of the existing tests break.

To test, just make a child directory with a gulpfile in and a couple gulp-* dependencies, and require gulp-load-plugins using '../'. Then it won't be able to find the dependencies until you merge this PR.

I can get rid of the editorconfig stuff if you want :)

jackfranklin commented 9 years ago

Thanks! I need to take some time to properly look at this, but will try to get back to you soon

jackfranklin commented 9 years ago

without adding a test dir and extra files.

This feels complex enough that if that stuff needs to be added, happy to do it.

callumacrae commented 9 years ago

I added a test. Running the test on master fails, running it on this branch succeeds. Cleaning it up and committing now :smile:

~/Sites/gulp-load-plugins config-fix*
❯ npm test

> gulp-load-plugins@0.8.0 test /Users/callumacrae/Sites/gulp-load-plugins
> mocha

  configuration
    ✓ throws a nice error if no configuration is found

  no lazy loading
    ✓ loads things in
    ✓ can take a pattern override
    ✓ allows camelizing to be turned off
    ✓ camelizes plugins name by default
    ✓ lets something be completely renamed
    ✓ does require at first

  with lazy loading
    ✓ loads things in
    ✓ can take a pattern override
    ✓ allows camelizing to be turned off
    ✓ camelizes plugins name by default
    ✓ lets something be completely renamed
    ✓ does not require at first
    ✓ does when the property is accessed

  requiring from another directory
    ✓ allows you to use in a lower directory

  15 passing (10ms)

~/Sites/gulp-load-plugins config-fix*
❯ git checkout -
M   package.json
D   test.js
A   test/index.js
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.

~/Sites/gulp-load-plugins master*
❯ npm test

> gulp-load-plugins@0.8.0 test /Users/callumacrae/Sites/gulp-load-plugins
> mocha

  configuration
    ✓ throws a nice error if no configuration is found

  no lazy loading
    ✓ loads things in
    ✓ can take a pattern override
    ✓ allows camelizing to be turned off
    ✓ camelizes plugins name by default
    ✓ lets something be completely renamed
    ✓ does require at first

  with lazy loading
    ✓ loads things in
    ✓ can take a pattern override
    ✓ allows camelizing to be turned off
    ✓ camelizes plugins name by default
    ✓ lets something be completely renamed
    ✓ does not require at first
    ✓ does when the property is accessed

  requiring from another directory
    1) allows you to use in a lower directory

  14 passing (28ms)
  1 failing

  1) requiring from another directory allows you to use in a lower directory:
     Error: Cannot find module 'gulp-test'
      at Function.Module._resolveFilename (module.js:338:15)
      at Function.Module._load (module.js:280:25)
      at Module.require (module.js:364:17)
      at require (module.js:380:17)
      at Object.defineProperty.get (/Users/callumacrae/Sites/gulp-load-plugins/index.js:56:18)
      at Context.<anonymous> (/Users/callumacrae/Sites/gulp-load-plugins/test/index.js:194:29)
      at callFn (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runnable.js:251:21)
      at Test.Runnable.run (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runnable.js:244:7)
      at Runner.runTest (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:374:10)
      at /Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:452:12
      at next (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:299:14)
      at /Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:309:7
      at next (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:248:23)
      at Object._onImmediate (/Users/callumacrae/Sites/gulp-load-plugins/node_modules/mocha/lib/runner.js:276:5)
      at processImmediate [as _immediateCallback] (timers.js:354:15)

npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0
callumacrae commented 9 years ago

Done. The commit cherry-picks fine onto master to test that it fails there (or I can send another pull request if you want Travis to run the tests).

callumacrae commented 9 years ago

Marked WIP: I hadn't thought about what would happen if the module to be required was further up the tree, too! This PR as is could break that as it's specifying a relative path.

callumacrae commented 9 years ago

The latest commit will fix an issue I mentioned in #56 where foo/bar/package.json wouldn't be able to require foo/node_modules/gulp-test. Works fine now.

callumacrae commented 9 years ago

@jackfranklin Hey, any update on this?

jackfranklin commented 9 years ago

Hey, I'm away on hols with limited internet at the moment. Sorry for such a low response on my end, I'll get back to you early next week.

callumacrae commented 9 years ago

No worries! Enjoy your holidays.

jackfranklin commented 9 years ago

@callumacrae nice stuff, thanks for the contrib! Few comments but nothing major.

Also make sure to add yourself to the package.json as a contributor.

callumacrae commented 9 years ago

I've made all the changed you requested :smiley:

jackfranklin commented 9 years ago

You're awesome, thanks man. I see you're London based so let's go for a pint sometime!

callumacrae commented 9 years ago

I'm at FEL, I'm the short guy :D

On 26 Feb 2015, at 20:26, Jack Franklin notifications@github.com wrote:

You're awesome, thanks man. I see you're London based so let's go for a pint sometime!

— Reply to this email directly or view it on GitHub.

jackfranklin commented 9 years ago

Well this just got weird... :P

callumacrae commented 9 years ago

Yeah, at FEL last month when they said you were speaking I was like "Woah, I just sent that guy a pull request!". Weird coincidence, I guess :)

On 26 Feb 2015, at 20:40, Jack Franklin notifications@github.com wrote:

Well this just got weird... :P

— Reply to this email directly or view it on GitHub.