Allow for multiple protocols to be resolved

[bashaus]

Hey revelrylabs!

I really love this script, it's great! Stumbled across it a few days ago.

In my projects, I use a combination of npm (backend stuff like reset-scss) and bower (mostly javascript dependencies) so I wanted to extend it to use multiple package managers. Rather than write another competing project, I wanted to contribute this back to your project. I hope you like it.

This process has made the options.prefix parameter deprecated, because ideally you'd use the registerProtocolResolver.

I also added in some functionality so that the main property of the package dependency is used if it exists - that way you don't need to know the specific implementation of the package.

Admittedly, some of the implementation is a little sloppy (sorry! I'm going to look to see if there's a better callback approach) - but I think the interface is pretty solid.

Would love to get your thoughts, and it would get great to see this form 2.1.0 of your project.

Kind regards! Bash

[bashaus]

@jwietelmann :)

[bashaus]

Updated the source so that it used a better approach for registering protocols. Cleaned up the source a little bit as well.

[jwietelmann]

@bashaus Hi! Thank you for this. I'm still kicking the tires on it. My knee-jerk reaction is: THANK YOU FOR WRITING TESTS. :100:

A couple things going on:

1. I'm cutting a new minor release with the #2 changes, since that fixes an edge case without changing the API.
2. I need to write a test case around #2 to ensure that follow-on PRs don't revert it.
3. I need to test your code against that case.

After that I'm going to look at your code a deeper dive. Just wanted you to know it's been seen.

[jwietelmann]

@bashaus So I was ready to be on board with this, but I did some research, and it appears that you can pass an array of importer functions to node-sass. Would it make more sense to just have multiple importers so people can pick and choose their own importers and prefixes? Do you want to make sassy-bower-importer and then work together to try to keep our APIs in vaguely in sync?

Because then I think all of this could just be...

var importFromNPM = require('sassy-npm-importer')();
var importFromBower = require('sassy-bower-importer')();

var result = sass.renderSync({
  file: __dirname + '/styles.scss',
  importer: [importFromNPM, importFromBower],
}

It would also give you the ability to give them the same prefix and choose which one gets first precedence over the other (whichever one comes first in the array). For example I could set up the pkg:// prefix to first look in Bower, and then if it doesn't find anything look in NPM:

const PREFIX = 'pkg://'
const importFromNPM = require('sassy-npm-importer')({prefix: PREFIX});
const importFromBower = require('sassy-bower-importer')({prefix: PREFIX});

var result = sass.renderSync({
  file: __dirname + '/styles.scss',
  importer: [importFromBower, importFromNPM],
}

[bashaus]

Sounds like a great idea, i didn't realise you could pass n array. Would be good though to keep some of the changes I've made, as it handles things like the main property, Travis ci, etc. I'll put something together and if you like it we can start the bower approach too.

[bashaus]

Hi

This pull request is if you'd like to go down the path of using the protocol registry. It's been amended to suit the changes you've made.

[bashaus]

So the reason I haven't removed this from the options is because I'm concerned about the amount of maintenance and repetition that is involved with maintaining two separate packages that essentially do almost the same thing. I was onboard with splitting the project, but after getting it working to this point - I'd like to suggest that we use pull request #3 instead of #4.

Of course, it's up to you :)

Let me know which one you choose.

[bashaus]

bump :)

[jwietelmann]

Sorry, I'm going to reject these. I still still think multiple packages is closer to the right approach, particularly because:

• I have no use for Bower, and even the Bower site recommends people start using npm/yarn instead. A lot of other people feel similarly, and I'd rather let them stay opted out of Bower things.
• I think the next step for this package is making it friendly for CLI usage, and it's unclear to me that this will help, while doing so with the current code is quite arbitrary.
• Taken with the above decisions, your protocol registry just adds a lot more code and architecture to a simple lib that will probably never need it.

If you write a Bower importer, I'm still willing to link to it on the README.