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

Is there any plan to create grunt plugin for sass.js? #61

Closed
amiramw opened this issue Jul 29, 2016 · 9 comments
Closed

Is there any plan to create grunt plugin for sass.js? #61

amiramw opened this issue Jul 29, 2016 · 9 comments

Comments

@amiramw
Copy link

amiramw commented Jul 29, 2016

No description provided.

@rodneyrehm
Copy link
Member

Nope. Maybe simple-sass (CLI for sass.js) helps?

@amiramw
Copy link
Author

amiramw commented Jul 31, 2016

I created an initial grunt plugin https://github.com/amiramw/grunt-contrib-sassjs. Hope it's ok. Not yet released to npm.
If you could review it would be great...

@rodneyrehm
Copy link
Member

@amiramw
Copy link
Author

amiramw commented Aug 8, 2016

Thanks for the feedback.
I think I covered most of the comments. Still need to look into the separate process thing.
Any more comments are welcome.

@rodneyrehm
Copy link
Member

Thanks for the feedback.

Thanks for your efforts!

Still need to look into the separate process thing.

I don't think that's the most important thing for now…

Any more comments are welcome.

Parsing and mutating paths

The importer callback looks like it's trying to mimic a subset of _libsassPathVariations(). Maybe we should talk about how we can expose this function so you can use it like _resolvePath()?

You want to use the Path module instead of playing the string manipulation game:

Error Handling:

SourceMaps

@amiramw
Copy link
Author

amiramw commented Aug 14, 2016

Fixed most of the comments. For now I copied _libsassPathVariations(). Let me know if you export it.

@rodneyrehm
Copy link
Member

Fixed most of the comments.

which ones didn't you fix and why?

Let me know if you export it.

As of 0.9.12 you no longer need your copy of _libsassPathVariations(), but can now obtain that list from Sass.js directly, see Resolving file names.


I would rewrite your source map handling to

var rootDirectory = "sass/" + PATH.dirname(src);
result.map.file = cssFile;
result.map.sources == result.map.sources.map(function (source) {
  return PATH.relative(rootDirectory, source).replace("\\", "/");
}),
grunt.file.write(cssFullPath + ".map", JSON.stringify(result.map));

Since you're using Sass.compileFile(), you don't need to filter stdin anymore. Since results.map already contains the map, there's no need to create a new object.

@amiramw
Copy link
Author

amiramw commented Aug 14, 2016

which ones didn't you fix and why?

At first I didn't populate the sourcesContent property as I understood it is optional in source map.

However I adopt your suggestion for JSON.stringify(result.map) and the sourcesContent is populated.
Also I now reuse getPathVariations from sass.js.

Thanks a lot for the help.

amiramw/grunt-contrib-sassjs@a65f869

@rodneyrehm
Copy link
Member

thanks for your efforts!

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

No branches or pull requests

2 participants