search

craigspaeth / nap

Compile, manage, & package stylesheets, javascripts, and javascript templates for node.js
MIT License
122 stars 33 forks source link

Nap should support async pre-processor APIs #49

Open craigspaeth opened 11 years ago

craigspaeth commented 11 years ago

Sass being one of them

ragulka commented 11 years ago

I just stumbled upon this as well. Looks like Less is actually async preprocessor as well. Speaking of which, the current less preprocessor implementation lacks support for @imports.

ragulka commented 11 years ago

@craigspaeth looking at the source code of nap, it seems that everything has been built around the idea of using synchronous functions:

  1. nap.package is synchronous, even though it provides a callback.
  2. fingerprintForPkg is sync,
  3. preprocessPks is sync,
  4. preprocess is sync,
  5. ... and middleware is the only one that is not synchronous.

What would be the easiest way to handle async preprocessors? I am seeing 2 possible ways:

  1. We can somehow make preprocess handle both async and sync results so that the rest of the chain (all the way down to nap.pakacge()) can be left unchanged.
  2. We need to rewrite all the functions so that they behave as async functions. This would mean that one needs to wait for the callback of nap.package() when using it inside app code.
ragulka commented 11 years ago

After playing around with various ideas and looking into possibilities for option 1, I don't think it's possible. There seems to be no easy way around this. Looks like supporting async preprocessors requires major rewrite of the existing code (including tests).

craigspaeth commented 11 years ago

Yeah I had feared a large rewrite would be necessary to support async pre-compilers. Sort of a pain, but I think ultimately necessary.

My only concerns are that it might complicate the setup or transition of nap, but I will look into it. Thanks for all of your help!

ragulka commented 11 years ago

I was playing around today to see what it would take. Started coming from preprocessors and got as far as preprocessPkg. Then I hit a roadblock.

fingerprintForPkg calls preprocessPkg, but the nature of fingerprintForPkg is completelt synchronous and it should stay so. Also, both css() and js() functions call preprocessPkg, but they are also synchronous in nature (they inject the necessary tags into views).

ragulka commented 11 years ago

It seems to me that there is a design/architectural issue here. As far as I understand, what NAP does is that in development mode, it does not preprocess any files unless they are requested via css(), js() or jst() functions. These functions look for the processed contents in the cache and if not there, will call the preprocessors. This allows assets to be re-processed on each request, if any of the file sin the package have been changed.

When it comes to other asset managers that I have tried (for example Asset Rack, which, while powerful, I find is not that easy to set up), it seems that they preprocess and set up the assets on app startup, and some of them watch for file changes in dev mode and then rebuild the assets.

To be honest, I like nap's approach more, but due to the async nature of node, it looks like it won't work out.

craigspaeth commented 10 years ago

Although this issue is a year old, it turns out LESS actually has a syncImport option, so now nap supports LESS commit here with imports. The problem of not being async still remains though :-/