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

Add importer registry to have multiple custom importers #962

Closed
mgreter opened this issue Mar 17, 2015 · 8 comments
Closed

Add importer registry to have multiple custom importers #962

mgreter opened this issue Mar 17, 2015 · 8 comments

Comments

@mgreter
Copy link
Contributor

mgreter commented Mar 17, 2015

Currently we only support one custom importer, that has the job to "convert" import urls to import-data (basically loading the content). To support multiple importers, each importer should have a function to determine if it wants and can handle a given import url. This way we could have multiple importers loaded, one for http traffic, another for ftp etc.

Idea

  • each importer implements a canHandle function (plus optional priority)
  • libsass asks each importer in priority order if it canHandle the import url
  • call importer that canHandle the url and repeat until first "valid" result

Some open questions

  • There is a chance that multiple importers can handle the same type of url (or one handles more specific urls than the other, like loading from a subdirectory). For this we probably need some kind of priority system and also an option to try other importers if current one gave an error. We could also accumulate all imports that get returned by multiple importers, but I don't really see a use case for that.
  • We could implement this on top of the currently existing API or change the existing API to support this kind of feature. Since it is still marked experimental, I'm propably for the second option.

Expose more stuff for importers (#984)

  • Expose function to resolve filenames with libsass (by using include paths)
  • Expose options of the active context (maybe pass ctx and use option API?)

This is open for discussion and more ideas!

@am11
Copy link
Contributor

am11 commented Mar 18, 2015

@xzyfer also mentioned this for node-sass on Gitter chat.

In JavaScript, the end-user can either use promises (such as async#waterfall: https://www.npmjs.com/package/async#waterfall, example usage: Bartinger/grunt-github-release-asset/github_release_asset.js#L42) or just a if/else, switch/case state machine to call appropriate function (to read data from database, web-service, file-systems etc.). For this, IMO, there is no need for API support and offloading the implementation design choice to final implementer would make sense: one generic entry point would suffice.

I may be missing the point. Please correct me if there is a use-case which cannot be handled without multiple importers.

@mgreter
Copy link
Contributor Author

mgreter commented Mar 18, 2015

The benefit if we provide our own implementation is mostly toward c++/c plugins (which can also be used by node-sass or any other libsass consumer btw). IMO it makes sense to have one API for this, since otherwise plugins may not be interoperable. But from an implementation standpoint, this functionality could be offloaded to a plugin, which in turn would handle the loading and calling of other plugins (they would only interact with the first plugin and not with libsass directly). IMO it makes more sense to bake this directly into libsass, since this layer of abstraction would not give much benefit. The current implementation would basically work the same, just adding one function that would signal it wants to handle every import it is given (and having the highest priority).

The idea is that you could create a http_imports.dll, which then could be used by node-sass, perl-libsass, sassc and every other consumer. We could have added css imports, sass imports etc. with such a dll. IMHO it makes absolute sense to evolve the current custom importer system in this direction!

@am11
Copy link
Contributor

am11 commented Mar 18, 2015

Does this mean libsass will define the types of custom importer? For instance:

http-handler: reads the resource from HTTP

https-handler: reads the resource from HTTPS

ftp-handler: reads the resource from FTP

network-fs-handler: reads the resource from network filesystem

local-fs-handler: reads the resource from local filesystem

custom-handler: reads the resource from data host, could be a web service call, database (@import my-connection-string#table 😎!) etc.

global-handler: reads the resource from all of the above resources. LibSass will always call this.

Q2: Would we be able to pipe the output of one importer to the input of another, with some kind of a precedence order? Calling multiple importers for same @import encountered. (this is a crazy use-case, but who knows someone may come up with this kind infrastructure which mandate this use-case..)

@mgreter
Copy link
Contributor Author

mgreter commented Mar 18, 2015

Each importer will first be asked if it is able to handle a certain url we encounter in imports (like plugin[0].canHandle('http://...')). This check is the function we would need to expect each importer to implement (so it can tell libsass which imports it can handle). I hope this makes it more clear!

@am11
Copy link
Contributor

am11 commented Mar 19, 2015

@mgreter, thank you for the explanation! 👍
On a related note, would it be possible to incorporate glob pattern, so implementer can supply a pattern to register against using can_handle()? Found this cross-platform globbing lib: http://www.shwild.org/ from http://stackoverflow.com/a/5213763/.

@mgreter
Copy link
Contributor Author

mgreter commented Mar 19, 2015

The canHandle function is part of the plugin itself. Libsass asks the plugin, if it can handle a certain url. So the import may look like this **/*.ccss (custom css). Your plugin could now say, ok, I can handle that url/path. Now it is in charge to actually do something with that url. IMO you could just expand that to a list of imports where you only set the filename, so libsass would take care of any further loading!

@mgreter mgreter added this to the 3.3 milestone Mar 22, 2015
@mgreter mgreter mentioned this issue Mar 25, 2015
@mgreter mgreter modified the milestones: 3.2.1, 3.3 Mar 28, 2015
@mgreter mgreter self-assigned this Mar 28, 2015
@mgreter mgreter modified the milestones: 3.2, 3.2.1 Mar 29, 2015
@mgreter
Copy link
Contributor Author

mgreter commented Mar 30, 2015

Merged #1000, which contains a first experimental implementation!

@mgreter mgreter closed this as completed Mar 30, 2015
@am11
Copy link
Contributor

am11 commented Mar 30, 2015

👍 ⚡ 🎉

I will start implementing it later this week (in the middle of moving), unless, of course someone else wants to give it a go! 😃 //cc @matryo 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants