jackfranklin / gulp-load-plugins

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

support for scoped packages #59

Closed ghost closed 9 years ago

ghost commented 9 years ago

The gulp-load-plugins module does not support scoped gulp plugins. npm-scope.

I believe line 20 could be changed from ['gulp-*', 'gulp.*'] to ['gulp-*', 'gulp.*', '@*/gulp{.,-}*'].

matijakovacevic commented 9 years ago

Have you tried adding it through options on initialization?

ghost commented 9 years ago

@matijakovacevic, that's not something I'd previously thought of doing, but I'm pretty sure I could use the built in options feature to set the plugin pattern, and load plugins from under a scope.

Though that's possible, I don't believe it's a viable solution. In my use case I'm not looking to support some arbitrary naming convention that myself, or maybe an organization, wishes to follow that diverges from the the conventions required by this module.

Instead, I believe this module should, out-of-the box, support npm scopes, as npm scopes are now a core feature of the npm ecosystem.

jackfranklin commented 9 years ago

@destroyerofbuilds completely agree that that should be a built in feature (sorry for the slow response, been on hols).

However, can you let me know if changing the config as @matijakovacevic suggests works? If so that's a really easy change to make and one I'd be quite happy to. Else I'll have to look further into it :)

ghost commented 9 years ago

@jackfranklin, I used gulp-load-plugins as follows:

var $ = require('gulp-load-plugins')({
    pattern: ['gulp-*', 'gulp.*', '@*/gulp{.,-}*']
});

I created a directory @myco in my node_modules folder and placed gulp-angular-templatecache under that directory. I included that package as @myco/gulp-angular-templatecache in my package.json file.

gulp-load-plugins found the package correctly. However, the requireName is really odd: @myco/gulpAngularTemplatecache.

Now I have to do $['@myco/gulpAngularTemplatecache'](options) to access that plugin.

I guess the replaceString will need to be updated to match scope names?

jackfranklin commented 9 years ago

Ah yeah, that's a bit funky. You'd probably want to make a similar change to replaceString to strip /@.+/\/ or similar out (my regex is a little off and I can't easily test right now).

ghost commented 9 years ago

@jackfranklin, what would you expect the name to be? Would the final, camelized, name be mycoAngularTemplatecache, or just angularTemplatecache? I ask, because I wonder if collisions between similar named scoped, and non-scoped, packages should be taken into consideration?

jackfranklin commented 9 years ago

@destroyerofbuilds hmm that's a good point, I had not considered scoping.

I wonder if they should be nested? Eg with these modules:

@myco/gulpAngularTemplatecache 
notScopedPackage

And:

var $ = require('gulp-load-plugins')();

Gives you:

$.notScopedPackage
$.myco.gulpAngularTemplateCache

What do you think?

ghost commented 9 years ago

@jackfranklin, that seems reasonable.

Though I would probably go for $.myco.angularTemplatecache.

jackfranklin commented 9 years ago

Ah yes - the camel casing stuff will be as is, that's just a typo on my part.

Do you fancy giving it a go in a PR? Else I'll try to take a look when I can :) Thanks for the discussion on this :+1:

jackfranklin commented 9 years ago

Actually, having taken a closer look at the npm docs:

As of 2014-09-03, scoped packages are not supported by the public npm registry.

If that's the case and npm isn't encouraging the use of scoped packages, I'm not so sure we need to be making the effort to support them here.

courtnek commented 9 years ago

@jackfranklin

From https://docs.npmjs.com/misc/scope :

As of 2014-09-03, scoped packages are not supported by the public npm registry. However, the npm client is backwards-compatible with un-scoped registries, so it can be used to work with scoped and un-scoped registries at the same time.

I think this implies that un-scoped registries are the 'backwards' registries. I think why scoping is unsupported in the public registry is because scopes exist to assisting in creating internal, non-public registries. I'd be :+1: for this request. Thoughts?

ghost commented 9 years ago

@jackfranklin, scopes are also already part of many in-house npm registry solutions, including npm Enterprise.

jackfranklin commented 9 years ago

@destroyerofbuilds @courtnek cool, thanks for correcting. In that case I'd be happy to see this in a PR :+1:

jackfranklin commented 9 years ago

Working on this now - will have a PR in shortly

ghost commented 9 years ago

Thank you @jackfranklin! Got tied up with other things. Would be happy to test and/or code review.