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

Remove synchronous compilation #333

Closed
jhnns opened this issue Dec 26, 2016 · 7 comments
Closed

Remove synchronous compilation #333

jhnns opened this issue Dec 26, 2016 · 7 comments

Comments

@jhnns
Copy link
Member

jhnns commented Dec 26, 2016

We currently have two code paths: one for synchronous compilation and one for asynchronous compilation. Since we learned that synchronous compilation can seriously slow down your build speed, we already discouraged the use of enhanced-require. In the next major release, we will completely remove the synchronous code path since it is a lot of work to maintain this second code path.

@gcarothers
Copy link

Can I recommend NOT removing this feature just yet. There are clearly some bugs in node-sass/libsass when loading sass from more than one entry point (or even two webpack builds in a single process) using the async callback. With even moderately complex sass files (bootstrap-sass) trying to use two entry points results in random Stack level too deep errors. In these cases setting isSync to true does "take longer" but results in NOT erroring out at random.

@benallfree
Copy link

benallfree commented Jan 2, 2017

Can confirm @gcarothers Stack level too deep, I get this one too when compiling all the bootswatch themes async.

@jhnns
Copy link
Member Author

jhnns commented Jan 2, 2017

Strange. I do have multiple entry points and I never experienced this Stack level too deep error. Would be nice to reproduce it reliably...

I'm thinking of moving node-sass into a separate process. Do you think this might solve it?

@benallfree
Copy link

I'd be happy to test it, maybe a separate branch? In the mean time, I'm going to use a parallel shell script to compile all the SASS into intermediate CSS and then webpack those. I'll report back to see if that works better.

@gcarothers
Copy link

sass/node-sass#1669 seems to be the root cause (changes to how node sass did memory management) hopefully node-sass will resolve soon. Until then downgrading back to older node-sass solves the stack level too deep errors completely.

@benallfree
Copy link

Using the node-sass 4.x CLI to build intermediate CSS files in parallel works while sass-loader fails in the same scenario. So it does seem that there is something sass-loader is doing differently than a bash script. Here is a code demo with YouTube walkthrough: https://github.com/benallfree/node-sass-1669

PS thank you @gcarothers I'll probably just downgrade to 3.x for the time being.

@macedd
Copy link

macedd commented Jan 30, 2017

Having issues with bootstrap 4 compilation.
Multiple entry points load its scss, so Stack level too deep is raised. Seems random..

isSync was removed.. how are you solving it @gcarothers ?

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

4 participants