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

Allow custom importers. #25

Merged
merged 1 commit into from
Jan 17, 2018
Merged

Conversation

jgerigmeyer
Copy link
Contributor

Fixes #24.

If a user passes in a custom importer option, this PR now passes that option through to node-sass. It also modifies the sass-extract specific importer to preprocess the filepath through any custom importers first.

node-sass accepts both sync and async versions of importer fns, and also accepts an array of fns or a single fn. This PR supports (and tests) all of those cases.

If the user, however, has an importer fn which returns { contents: <...> } (instead of { file: <...> }), that is not supported. It will still pass through to node-sass (so the files can be compiled properly), but sass-extract won't be able to extract variables from those imports. I think those would be unusable by sass-extract in any case?

@jgerigmeyer
Copy link
Contributor Author

Updated the PR to support custom importers which may return absolute import paths.

@jgerigmeyer
Copy link
Contributor Author

@jgranstrom Any thoughts on this?

@jgranstrom
Copy link
Owner

@jgerigmeyer thank you for this, and sorry for the late reply. There should definitely be support for providing custom importers, so this is great.

I will merge this and include it in a new release, however before that it would be great if you can amend the commit messages and change the message format to the convention used in all other commits and mentioned in the readme to keep it consistent and generate changelogs. There is some details on that process available.

@jgerigmeyer
Copy link
Contributor Author

@jgranstrom Rebased with new commit message. Let me know if that looks okay!

@jgranstrom
Copy link
Owner

@jgerigmeyer perfect, thank you! I’m currently a bit off the grid but will try to release this later today!

@jgranstrom jgranstrom merged commit afd4788 into jgranstrom:master Jan 17, 2018
@jgranstrom
Copy link
Owner

I published this in version 2.1.0. Thank you for the contribution. Make sure to get back to me if you find any other features missing for your use cases.

@jgerigmeyer
Copy link
Contributor Author

Thanks!

@jgerigmeyer jgerigmeyer deleted the custom-importer branch January 17, 2018 18:58
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

Successfully merging this pull request may close these issues.

2 participants