-
Notifications
You must be signed in to change notification settings - Fork 23
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
Bump node-sass@v4.0.0 #40
Conversation
Even if the major has been incremented, it's BC with v3.13.1 as explained in the [release notes](https://github.com/sass/node-sass/releases/tag/v4.0.0).
👍 |
Thanks! Will get this merged in asap :) |
@stevenschobert I also have made this commit (and use my fork ATM) https://github.com/shouze/metalsmith-sass/commit/6a4a879f787bb7bfd07f3bde739a4fadd1c1dfe8 because of some issues that can occur with node-sass >= 4.0.0 see sass/node-sass#1669 (comment) |
@shouze Oh interesting. Are you seeing that issue a lot, or just one particular edge case? I skimmed through the thread but curious what your experience with it was. If there are issues with 4.0.0, it might be better to wait for a couple patches to be out before updating metalsmith-sass. |
@stevenschobert this occurs everytime on the app I've upgraded so yes it probably worth to wait... or use renderSync like in the commit I've made. |
Heads up fix for the node-sass issues with land in 4.2.0 in the next 48 hours. |
Oh awesome @xzyfer! Thanks for the heads up (and all the hard work you do on libsass and node-sass, and everything else. Seriously! 🍻) |
<3 thanks @stevenschobert |
node-sass@v4.2.0 was released a couple hours ago. It should resolve this issue. |
@xzyfer I've just seen, that, testing again with |
I'm jumping into bed now. I'll check back for updates in the morning. |
@xzyfer it's perfectly working, thx for the great work! ping @stevenschobert so it's ok to merge from this time ;) |
Thanks everyone, will have this merged in shortly! |
@stevenschobert 👍 can't wait fot the new tagged release! 👼 Do you want some help on this repo to have:
|
Release is up! v1.4.0 Let me know if anyone has issues with it! |
@shouze For the travis build with multiple node versions, I'd love to see a PR! That'd be great. For using binaries, what did you have in mind? Might want to open up a separate issue on it so we can talk it out first. |
@stevenschobert sorry... but yes still having some random issues @xzyfer I can try to bring you some app that fail if you want, this is the same old @stevenschobert about binaries, this is just that every node-sass release bring binary binding that are downloaded during node-sass install so it's not required anymore for supported node releases & platforms to compile node-sass anymore in fact. |
@shouze sorry I don't follow. Are you saying you're still experiencing issues with stack level too deep? If so please file an issue with node-sass with a reproduction. It's entirely possible you're actually violating the maximum stack depth of 500. |
@xzyfer yes that's it and yes, probably violating the maximum stack depth of 500 (unfortunately the error occurs on a static website generation that uses one the the latest bootstrap alpha releases). I'll open a new issue ASAP, my turn to go to bed for now. |
Interesting. I'll try compiling the bootstrap alpha release tonight. I can't imagine they're exceeding the limit without them noticing until now. It's likely the combination of mixins being used. |
Even if the major has been incremented, it's BC with v3.13.1 as explained in the release notes.