search

Cowboy-coder / bundle-up

An asset manager for nodejs
MIT License
56 stars 22 forks source link

Any plans for less support? #10

Closed panosru closed 12 years ago

panosru commented 12 years ago

As title says... are there any plans to support less ?

Cowboy-coder commented 12 years ago

I'd gladly accept pull requests for it. :) I don't use Less so right now I have no plans for it.

panosru commented 12 years ago

I'm gonna work on that then :)

Cowboy-coder commented 12 years ago

Cool! Make sure to write some tests for it :)

You can run the tests using ./node_modules/.bin/jasmine-node --coffee specs/

panosru commented 12 years ago

@Cowboy-coder could you point me on something that I scratch my head a little bit :P

I get 

Error: ENOENT, no such file or directory 
/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/public/generated/front-end/assets/css/bootstrap/bootstrap.css

and I'm wondering why the module does not generate this file... if you could point me on where to look would be great

Cowboy-coder commented 12 years ago

What version of node are you using? I would recommend to use 0.6.x. Is it a test that fails? In what context does this happen?

panosru commented 12 years ago

I use 0.6.9 and not it's not the test that fails but the script itself, I created the file manually to see if the whole parsing process with less works and it works fine but the module does not generate the file for me. I have set assets.addCss('css/bootstrap/bootstrap.less') and I'm not sure where exactly to look, from what I see it should work out of the box, shouldn't it?

This is the full error log:

Error: ENOENT, no such file or directory '/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/public/generated/front-end/assets/css/bootstrap/bootstrap.css'
    at Object.openSync (fs.js:230:18)
    at Object.readFileSync (fs.js:120:15)
    at /home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:152:23
    at Css.<anonymous> (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:173:36)
    at Css.toBundles (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:3:61)
    at new BundleUp (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle_up.coffee:38:18)
    at /home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle_up.coffee:59:12
    at Object.<anonymous> (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/index.js:28:1)
    at Module._compile (module.js:444:26)
    at Object..js (module.js:462:10)
Cowboy-coder commented 12 years ago

No, .less files shouldn't work out of the box. But are you sure that assets.addCss('css/bootstrap/bootstrap.less') is the right path to the file on your file system? I think it might be a problem related to .less files though. Try to rename bootstrap.less to .styl just to see if it's a problem related to .less files.

panosru commented 12 years ago

@Cowboy-coder I thought that once I add an extension to be supported by css or js then the module would automatically generate an empty file mapped to the path and name of that file with the properly appended extension (js or css). If you want to take a look with what I got so far you can do it here

If I create an empty file under generated folder that will map css/bootstrap/bootstrap.less then everything works fine, the less file got compiled and written to the file normally, the only obstacle now is that the css file is not generated by the module before the compilation process.

Thanks

panosru commented 12 years ago

So the issue is because of bundling, for some reason I get this error when I got bundle : true I'll investigate it :)

Could you let me know how should I test otf_compiler.coffee file?

panosru commented 12 years ago

If I use bundle : false and run the application files are generated and things work properly, then if I change bundle : true and run the application again, files are bundles and things still work properly, if I remove the generated folder and then I try to run the application having bundle : true then I get the error, I guess this happens because when the module try to bundle up scripts the less -> css file is not yet generated as a result of the error...

Cowboy-coder commented 12 years ago

I wasn't able to reproduce it locally. Maybe you don't have rights to create the generated/ folder? If you could send me a reproducable example I could try it out. Regarding otf_compiler tests, you can do it anyway you like. Just make sure that the compiling actually occurs when the .less file has been changed. It doesn't have to be anything fancy. Also, if possible the css should also be re-compiled when an imported .less file changes. This can be a bit tricky. At least it was a bit tricky for stylus.

panosru commented 12 years ago

@Cowboy-coder yes the generated has permissions to be created since the application works fine if I don't set bundle : true. The application that I do tests on is here and the forked version of bundle-up with less support is here.

What is happening is that:

  1. if I set bundle : false and start the application everything works fine, files are generated properly etc.
  2. if I set bundle : true and have generated files ready from previous start with bundle : false then the files are bundled in one file and everything works properly again.
  3. if I set bundle : true and don't have generated files ready then I get the error that I posted previously, it is like the bundling process start before the files are generated.
panosru commented 12 years ago

I tried to use tests in order to make debugging simpler but when I run npm test I or the command you provided for running tests I get the following:

root@dev:/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up# npm test

> bundle-up@0.2.5 test /home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up
> node_modules/.bin/jasmine-node --coffee specs/

Finished in 0 seconds
0 tests, 0 assertions, 0 failures

Output with verbose:

root@dev:/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up# npm test

> bundle-up@0.2.5 test /home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up
> node_modules/.bin/jasmine-node --coffee --verbose specs/

undefined

Finished in 0.006 seconds
0 tests, 0 assertions, 0 failures
Cowboy-coder commented 12 years ago

Not sure why the tests doesn't run. But afaik it should work out of the box since it works on both my machine and on Travis CI.

I will check your re-producable example later

panosru commented 12 years ago

I believe, the reason that bundle is not working is because less is taking too long to compile the file...

I replaced this:

    when 'less'
      compilers.less(content, file).parse content, (err, tree) ->
        css = tree.toCSS()
        if err?
          console.log err.message
          css = err.message
        return cb(err, css)

with this:

    when 'less'
      return cb(null, 'panosru')
      ###
      compilers.less(content, file).parse content, (err, tree) ->
        css = tree.toCSS()
        if err?
          console.log err.message
          css = err.message
        return cb(err, css)
      ###

and it worked like a charm...

EDIT: So the error is caused when I use @import in my less files... If you could suggest me something would be cool, I will continue spend some more time on it because is vital feature for my needs...

Cowboy-coder commented 12 years ago

So, yeah I have managed to reproduce it, but I can't seem to understand why it breaks as soon as @imports are used. I don't get any hints from any of the error parameters either so it's difficult to understand the problem. Sorry for not being much of a help. Maybe you should ask in Less issue tracker if you are using the API incorrectly or something.

Btw, I have switched jasmine to mocha. The new way to run the tests (in master) is ./node_modules/.bin/mocha

Cowboy-coder commented 12 years ago

Nvm, I have found the issue. I will fix it in the upcoming days. The bundling implementation needs to change a bit in order to support Less compiling. The reason it doesn't work right now is because when the bundling occurs the Less compiler hasn't compiled the Less file yet (because it happens asyncronously). So the error that has been thrown all the time isn't from the Less compiler or something like that. It's actually from fs.readFileSync(...) in the bundling function, because it can't find the output file from the .less-file.

panosru commented 12 years ago

@Cowboy-coder Hello :) This is what I try to explain so long :D great I'm exited that you managed to track it :) The way I use less is correct, I tried multiple ways and they are all correct but I couldn't find a way for synchronous compiling in order to avoid the modification of bundle-up bundling mechanism but it seems there isn't any synchronous way to compile less files, but of course if you change the mechanism to be compatible with async compiling then would be easier to extend many other things.

EDIT: There are already some threads related to synchronous parsing in less here

panosru commented 12 years ago

@Cowboy-coder Regarding the compiling of less since it is async, do you believe that TameJS could help?

panosru commented 12 years ago

I tried to use await for the compiler by using iced-coffee-script which seems to be cool but I was getting

TypeError: Cannot call method 'charAt' of undefined
    at Object.normalize (path.js:296:27)
    at Js.<anonymous> (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:122:19)
    at Js.addFile (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:3:61)
    at Js.<anonymous> (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:230:28)
    at Js.toBundles (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle.coffee:3:61)
    at new BundleUp (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle_up.coffee:39:17)
    at /home/rnd/domains/design.rnd/public_html/app-ui/apps/main/node_modules/bundle-up/lib/bundle_up.coffee:61:12
    at Object.<anonymous> (/home/rnd/domains/design.rnd/public_html/app-ui/apps/main/index.js:40:1)
    at Module._compile (module.js:441:26)
    at Object..js (module.js:459:10)

unfortunately I don't have much time currently to debug that so I stay with bundle : false for the moment :) but I believe iced could help on that instead of refactoring the compiler completely :)

cheers

panosru commented 12 years ago

I believe since less compiler is async we can't do much here.. two only options are either less will provide a sync compiler (which I don't believe they will do because there are already requests for this but seems that they don't care...) or bundle-up bundler will be async.

What's you opinion on this topic? Do you believe bundle-up could bundle things in async way?

One thing I noticed is that with bundle : false everything works fine with less, so I'm wondering if some-how the bundling could be triggered once files have been generated.

panosru commented 12 years ago

Unfortunately iced-coffee-script didn't helped, it could help if less compiler was using it... what a failure by lesscss that they don't provide a sync way to compile less files... many peoples bump on this issue and they had to drop the support for less... connect-assets also had the same issue...

For now what I do is 1) I start the application with bundle:false in order for the less files to be compiled (because I don't change them often they are the less files from twitter's bootstrap and jquery ui bootstrap) and then I start again my application with bundle:true since less files are already compiled in css and no further modifications needed on those files then the bundling process works fine. Of-course this is not a solution :P

Cowboy-coder commented 12 years ago

A somewhat late answer to this. But yeah, as long as Less doesn't have synchronous compiling it will not work well with this library. I tried doing a small re-write to support asyncronous compiling a while ago, but it will only make the library buggy because the compiling needs to happen on startup before the server starts. If not users will start to request .js and .css files not yet compiled.

I will close this ticket. But if Less in the future supports syncronous compiling then I'll gladly accept pull requests for it.