Support importer option

[ncoden]

Hi @theJian,

Do you know if you could add the support of the importer option ?

This option would be useful to use gulp-shopify-sass with eyeglass.

[theJian]

@ncoden If you think some features may be helpful you can add it by yourself. Let me know if you will add any feature, I can send a collaborator invitation to you. 😄

[ncoden]

Hi @theJian. I am currently working on this feature. But I notice it require to support the asynchronous render. It would be really helpful if you could uncomment/clean the current commented code about it.

[ncoden]

Also, I am using gulp-shopify-sass to simply concat Sass files, not for shopify. Do you think you could make this package generic, with some options that would make it usable for a shopify case ? I think there is only -with the name of the package- the extension substitution by .cat.scss.liquid that is not generic.

An other solution is to provide two packages: a generic one gulp-sass-concat, and gulp-shopify-sass that uses the first one but with Shopify-specific code and options.

[ncoden]

I also need to know: are you reviewing the PR I make ? Can I assume the changes I introduce will be verified ? Or should I be careful because of you do not have the time to maintain this package and you cannot ensure that my changes do not introduce bugs ?

[jasonyuezhang]

Hi @ncoden, I was the one who brought up idea of the async render feature. I would love to resume the development of this project in one week. I will need some time to pick the whole project up so if you have an idea of how to continue the development, please let me know of we could avoid the overlap.

[jasonyuezhang]

@ncoden I assume what you mean by making this plugin more generic is that you want to involve more options to make it work like node-sass. unfortunately, compare to the node-sass, this project is pretty immature. And I couldn't see there's way to improve the usability of this project significantly in a short period of time. Maybe some time in the future we will come up with a development schedule. However, for the current shopify project you might work on, don't rely on this plugin or I assume you will be stuck at nowhere.

[ncoden]

Hi @zhangyue1208. I need the async render feature because of the importer option is a callback which provide a done function, to call with the new url of content of the file. I imply that importReplacer should be async, because of it have to wait for this function to be called to continue importing files. So for now I don't need the support of all aync options, I only need the internal functions to works asynchronously.

[jasonyuezhang]

@ncoden sure I will see what I can do. However, please be aware of that we are all contributing on this project at our spare time and there's no guarantee of developing progress.

[ncoden]

@zhangyue1208 I am not working on a Shopify project. I am only using this package to concat Foundation Sass files into one distribution files with Sass dependencies included. It works well for now. I need this package to support the importer options because of eyeglass uses it.

gulp.src(...)
.pipe(gulp-shopify-sass(eyeglass()))
...

Like I explained above, making this package generic is not difficult and require (I think) only a generic name (obviously) and to not replace the extension with .cat.scss.liquid.

[jasonyuezhang]

@ncoden from my understanding, you only need files to be concatenate into a whole piece without compile it?

[ncoden]

@zhangyue1208 Yes. But it is exactly what is package is doing, among the .cat.scss.liquidextension and the search for .scss.liquid files, isn't it ?

[jasonyuezhang]

@ncoden You are absolutely right. I just recalled. Thanks for the explanation!

[ncoden]

@zhangyue1208 So I just need for the commented code to be cleaned, because of I don't know what is related to the async feature and what it is not. But if you cannot do it for now, it's not a problem, i'll take care of it later.

[theJian]

@ncoden I haven't used this package for a long time. So, yes, any changes should be careful.
@zhangyue1208 Would you be a collaborator to keep developing this project.

[jasonyuezhang]

@theJian Sure I would love to.

[ncoden]

@theJian I would like too 😄

[jasonyuezhang]

@ncoden I will try accomplish the async render today. If I couldn't achieve it, I will let you know where I left and you can continue form there.

[theJian]

I have send invitation to you guys. Thanks.

[jasonyuezhang]

@theJian thank you for bring it alive and all the maintaining.

[jasonyuezhang]

@ncoden I've add async render feature. Just noticed you didn't add any test for includePaths. Do you mind add some simple test?

[ncoden]

@zhangyue1208 We have write access, but I think we should still use branches (see http://nvie.com/posts/a-successful-git-branching-model/) and pull requests, to review each others works.

[jasonyuezhang]

@ncoden cannot agree more, I will follow the pattern from now on. Thanks for the amazing advice!