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

LESS support (latest version, for twitter bootstrap 2) #30

Open
lnbogen opened this issue May 31, 2012 · 52 comments
Open

LESS support (latest version, for twitter bootstrap 2) #30

lnbogen opened this issue May 31, 2012 · 52 comments

Comments

@lnbogen
Copy link

lnbogen commented May 31, 2012

Hi dirkmc,

Until now, I worked with less 0.9.1.
I want to use press but I have conflicts with versions (less 0.9.1 links to rhino-1.7R3.jar).

Any idea when a new version that supports less, with the latest engine (lesscss-engine-1.3.0.jar) ?

Thanks,
Oren

@dirkmc
Copy link
Owner

dirkmc commented Jun 1, 2012

Hi, I'm planning to make press much more modular so it will be possible to add support for other engines as well, such as stylus and sass.
I'm going to say that I'll do it by the end of the week to put a bit of pressure on myself, otherwise I'll never get round to it. Please complain if it's not done by then :)

@lnbogen
Copy link
Author

lnbogen commented Jun 1, 2012

That would be amazing! Thanks!

@lnbogen
Copy link
Author

lnbogen commented Jun 4, 2012

Hi dirckmc, (quietly) complaining :)

@dirkmc
Copy link
Owner

dirkmc commented Jun 4, 2012

When I said the end of the week, I meant the end of this week :)

@lnbogen
Copy link
Author

lnbogen commented Jun 11, 2012

Hi dirckmc, is there a way to do a quick get-around myself (at least to make it to play nicely with less 0.9.1)?

@dirkmc
Copy link
Owner

dirkmc commented Jun 11, 2012

Hi, I'm working on making the change now. Hopefully it will be done by tomorrow.
Dirk

@dirkmc
Copy link
Owner

dirkmc commented Jun 12, 2012

I've deployed the latest release to the module repo, version 1.0.27.
Please try it out and let me know how you go.

@lnbogen
Copy link
Author

lnbogen commented Jun 12, 2012

Great, thanks! Will report...

@lnbogen
Copy link
Author

lnbogen commented Jun 12, 2012

I still see that you're using and old less engine (we're using lesscss-engine-1.3.0.jar to support twitter's bootstrap 2).
In addition, there is still clash with less 0.9.1 module. As I've mentioned, less 0.9.1 uses rhino-1.7R3.jar and there is some conflict. What do you recommend doing?

@lnbogen
Copy link
Author

lnbogen commented Jun 12, 2012

[some background]
An easy way to reproduce it is to set up an application with dependencies.yml to include both press 1.0.27 and less 0.9.1.
We're pretty happy with less module for the css part, it handles combining, minification is less important due to gzip.
We want to use press to handle our js combining/minifcation (and obviously versioning).

@dirkmc
Copy link
Owner

dirkmc commented Jun 12, 2012

Oh I didn't realize I was using an old less engine, I just used the one that another person who made a pull request put in there. If I used a newer version of the less engine jar will that solve the problem? Or will it still have a conflict with less?
I guess in that case you could just use press instead of the less module

@dirkmc
Copy link
Owner

dirkmc commented Jun 12, 2012

Ok I've updated the version of the less library to 1.3.0. Try out the latest version of press, 1.0.29

@lnbogen
Copy link
Author

lnbogen commented Jun 12, 2012

were you able to run a local application with both dependencies:

  • play -> less 0.9.1
  • play -> press 1.0.29

?

@dirkmc
Copy link
Owner

dirkmc commented Jun 12, 2012

I haven't tried it.. did you try it and have a problem? I've updated to less-engine-1.3.0

@dirkmc
Copy link
Owner

dirkmc commented Jun 12, 2012

I tried less-0.9.1 and press-1.0.29 and it seems to work fine

@lnbogen
Copy link
Author

lnbogen commented Jun 12, 2012

Alright, then I'll try to test everything locally with bootstrap.less file in the and see if it's good.

@lnbogen
Copy link
Author

lnbogen commented Jun 13, 2012

I get a strange behavior.
I've fetched less module from github (https://github.com/lunatech-labs/play-module-less)
Once I download it, I go to play-module-less\samples-and-tests\bootstrap-sample and configure dependencies.yml with both less 0.9.1 and press 1.0.29. Run play deps --sync.

Open the homepage (where it has bootstrap.less file that should run via less module, not press!) - you'll see that the css returns with "\n" (as if press changed it) which is corrupted. I've made sure (application.conf) that press.enabled was set to false and didn't add it to routes. The plugin should not have worked and yet it somehow change .less file.

In addition, removing press 1.0.29 from the dependencies solves it.
Maybe by adding press 1.0.29 it somehow makes less module misbehave?

@dirkmc
Copy link
Owner

dirkmc commented Jun 13, 2012

Hey Oren,
That is strange. I tried out a sample less file with the less module, with press, and with a combination of the two. It all works fine for me, the less files are compiled to css on the fly, and the press files are compressed without a problem.
I'm not sure what else could be going wrong.

@lnbogen
Copy link
Author

lnbogen commented Jun 17, 2012

Dirk, can you please pull https://github.com/lnbogen/bootstrap-sample and see test it there?

I get -

@6alf7a98g
Internal Server Error (500) for request GET /press/js/DXbXyueZQbqCpnMGQZgqrAJJ.js

Execution exception (In {module:press-1.0.29}/app/press/ScriptCompressor.java around line 84)
NoSuchMethodError occured : org.mozilla.javascript.Parser.parse(Ljava/io/Reader;Ljava/lang/String;I)Lorg/mozilla/javascript/ScriptOrFnNode;

play.exceptions.JavaExecutionException: org.mozilla.javascript.Parser.parse(Ljava/io/Reader;Ljava/lang/String;I)Lorg/mozilla/javascript/ScriptOrFnNode;
at play.mvc.ActionInvoker.invoke(ActionInvoker.java:231)
at Invocation.HTTP Request(Play!)
Caused by: java.lang.NoSuchMethodError: org.mozilla.javascript.Parser.parse(Ljava/io/Reader;Ljava/lang/String;I)Lorg/mozilla/javascript/ScriptOrFnNode;
at com.yahoo.platform.yui.compressor.JavaScriptCompressor.parse(JavaScriptCompressor.java:312)
at com.yahoo.platform.yui.compressor.JavaScriptCompressor.(JavaScriptCompressor.java:533)
at press.ScriptCompressor.compress(ScriptCompressor.java:84)
at press.Compressor.compress(Compressor.java:164)
at press.Compressor.writeCompressedFile(Compressor.java:143)
at press.Compressor.getCompressedFile(Compressor.java:123)
at press.Compressor.getCompressedFile(Compressor.java:91)
at controllers.press.Press.getCompressedJS(Press.java:29)
at play.mvc.ActionInvoker.invokeWithContinuation(ActionInvoker.java:551)
at play.mvc.ActionInvoker.invoke(ActionInvoker.java:502)
at play.mvc.ActionInvoker.invokeControllerMethod(ActionInvoker.java:478)
at play.mvc.ActionInvoker.invokeControllerMethod(ActionInvoker.java:473)
at play.mvc.ActionInvoker.invoke(ActionInvoker.java:161)
... 1 more

@dirkmc
Copy link
Owner

dirkmc commented Jun 18, 2012

Hey Oren,
Thanks a lot for taking the time to help out with debugging. It seems that both yui-compressor and lesscss-engine rely on Rhino (a Java -> JavaScript bridge). The problem is that yui-compressor takes the rhino jar, uncompresses it, and then modifies the classes, to produce a new Rhino jar. As far as I can tell the modified version of Rhino is not compatible with what lesscss-engine needs.

I'm not really sure how to get around this problem. Is it possible for you to use press to compress the less files instead of less?
Dirk

@lnbogen
Copy link
Author

lnbogen commented Jun 18, 2012

Tried to remove less and use press only (you can pull again, I've pushed the change).
Also - did play deps --sync and play clean to make sure nothing is being saved locally other than press.
Anyway, as you can see, the returned css is malformed..

@lnbogen
Copy link
Author

lnbogen commented Jun 18, 2012

In addition, I see you're using 2.4.6 version of yui-compressor. Thinking of moving to 2.4.7? Might solve known bugs and dependency issues?

http://www.yuiblog.com/blog/2011/11/14/announcing-yui-compressor-2-4-7/

@dirkmc
Copy link
Owner

dirkmc commented Jun 18, 2012

Ugh this dependency stuff is such a pain. Ok I'll try again and see if I can find the problem with CSS compression.

With regards to your question about moving to a later version of yui-compressor:
I tried compiling yui-compressor from source with the same version of Rhino that lesscss-engine uses, but I had the same problem because yui-compressor actually overwrites the Rhino classes and makes its own version of the jar. It's this modified Rhino jar that conflicts with the normal Rhino jar that lesscss-engine includes.

@dirkmc
Copy link
Owner

dirkmc commented Jun 18, 2012

By the way in main.html line 108 there's a space before the closing quote that causes an error (on the Mac at least):
#{press.script 'bootstrap-transition.js ' /}

Also there's no need to check in /public/bootstrap/js/press, this is a temporary directory generated by press on the fly.

@lnbogen
Copy link
Author

lnbogen commented Jun 18, 2012

about the bug - thanks! fixed.
I need to update my gitignore although it's not critical, this is just a demo app :)

Regarding the dependencies - it's a mess indeed.
This should be solved so both modules could co-exist as they offer different behavior (for example, the less module cache strategy is better as it checks for internal dependencies via "import *.less" files).

Sorry I cannot help more

@dirkmc
Copy link
Owner

dirkmc commented Jun 18, 2012

Ok I've upgraded the version of yui compressor to 2.4.7, it seems to be working a bit better now, at least the files are not empty and there are no exceptions, although when I tried it with your project it seems like there was some weird escaping going on with \n.
Try out version 1.0.30 of the plugin and let me know if it works better for you.

@lnbogen
Copy link
Author

lnbogen commented Jun 19, 2012

Yea, I saw the "\n" thus "as you can see, the returned css is malformed.." :/

Notice that it happens regardless of the less module (which is not installed). Obviously, there is something going on with the press module and less file.

@dirkmc
Copy link
Owner

dirkmc commented Jun 19, 2012

I don't have the issue with CSS in my own tests, I wonder if the less files have a weird encoding? I'm using a Mac and you're using Windows, right? Perhaps that makes a difference with the latest version of lesscss-engine. Are you using any special configuration options with the less module?
I'm going to check out the source code to see if there are any differences.

@lnbogen
Copy link
Author

lnbogen commented Jun 19, 2012

I'm using windows...
You need to test it with less (try the new syntax by bootstrap 2, this is what I've used for my demo).

No special configuration option for the less module.

@lnbogen
Copy link
Author

lnbogen commented Jun 19, 2012

Some strange behavior I see (maybe due to how play/modules work that I do not understand).
Tried to:
use less via dependency: play -> less 0.9.1
use press as local repository (after doing pull of your latest commits)

Then it works... How is that possible? Different behavior for local repository module?
So much voodoo with these dependencies on Rhino.

@dirkmc
Copy link
Owner

dirkmc commented Jun 19, 2012

I looked at the source code for the less module, and also the less engine itself. I don't think the \n bug is caused by anything on the Java side, it seems to be something within the JavaScript itself. I'm going to make a workaround for the bug, and also rework the implementation so that it handles imports and so on. It should be done by this afternoon.

@dirkmc
Copy link
Owner

dirkmc commented Jun 19, 2012

With regards to your questions about the way that dependencies are imported, I agree it just seems to be voodoo with the Rhino dependency. I think probably it depends on the order in which jars are added to the classpath.

@lnbogen
Copy link
Author

lnbogen commented Jun 19, 2012

Great, let me know and I'll test it on my demo + real application. Thanks!

@dirkmc
Copy link
Owner

dirkmc commented Jun 19, 2012

Ok try out press version 1.0.31
Thanks again for your help on this

@lnbogen
Copy link
Author

lnbogen commented Jun 20, 2012

Hi dirk, please pull again. Still no luck, getting "\n" in the .less files..

@dirkmc
Copy link
Owner

dirkmc commented Jun 20, 2012

Is the file still cached? Try making a modification to the file (eg add and then delete a space) and then save it.

@lnbogen
Copy link
Author

lnbogen commented Jun 20, 2012

Nope, not cache related...
Did you pull? it doesn't happen on your machine?

@dirkmc
Copy link
Owner

dirkmc commented Jun 20, 2012

I was using my local copy of press, but when I tried with the version I uploaded to playframework.org I saw the same problem as you. Anyway I've uploaded and tested a new version, press 1.0.32 and it works for me.
Please give it a shot and let me know if it works for you too.

@dirkmc
Copy link
Owner

dirkmc commented Jun 20, 2012

PS don't forget to modify the files or it will use the cached copy

@lnbogen
Copy link
Author

lnbogen commented Jun 20, 2012

Perfect, works now!
What did you change on the upload to playframework.org?

@dirkmc
Copy link
Owner

dirkmc commented Jun 20, 2012

As far as I can tell I didn't change anything. I think maybe the last time I uploaded it picked up the old files somehow, I'm not really sure. Anyway I'm glad it's working. Please let me know if you run into any other bugs as you're developing. And thanks again for all the help!

@lnbogen
Copy link
Author

lnbogen commented Jun 20, 2012

Cool, will let you know! thanks again for your effort!

@iamaracinghorse
Copy link

Hey folks! I know this is a months-old issue, but I was wondering if either of you ever gained any insight into the whole '\n' thing. I am having a problem wherein the un-minified versions of the less files are being returned with these bad line-breaks. BUT! The weird thing about it is that it's only happening on my teammates' machines and not my own. We're all using macs with the same jvm version, etc...

It's super weird. Just wanted to see if you had come across any solutions. I'm using press 1.0.35 (it was happening on 10.33 as well) and less 0.9.1, for what it's worth. OK! Thanks!

@lnbogen
Copy link
Author

lnbogen commented Aug 9, 2012

Sadly, I tried to integrate it with no luck so far.
On a side-project that I tested it on it seemed to work (with 1.0.35) but trying to integrate it within our project caused some errors (collision of libraries with LESS module and "\n" as you stated).

It looks like someone needs to take the all thing and create one package to rule them all or we'll always suffer from collisions between PRESS and LESS internal libraries. We currently have only LESS running =(

@dirkmc
Copy link
Owner

dirkmc commented Aug 9, 2012

Ugh damn it. Sorry I thought this issue was resolved.

The problem seems to be that there are two versions of the Rhino jar. One is the standard Rhino included by less-cssengine. The other is a hacked Rhino jar included by yui-compressor. It seems fairly random as to which gets picked up by the JVM.

Could you try doing the following:

  1. Replace references to less files with a press tag:
<link href="/public/stylesheets/main.less" rel="stylesheet"></link>
#{press.stylesheet 'main.less' /}
  1. Remove dependencies on the less module from config
  2. Delete the less module from your play install. I think it's under play/modules/less-0.9.1
  3. Run play clean

@iamaracinghorse
Copy link

Thanks for the response! Unfortunately, if I remove the less module, a couple of things happen:

  • When press.enabled is 'false' or when in dev mode, the less file is being served with a plain text mime type, so it's not being interpretted as a stylesheet. I can add a mimetype to the conf file that successfully allows play to serve is as text/css, but...
  • When press.enabled is 'false' or when in dev mode, the less files are not getting processed by the less compiler (this works when the less module is present though).

I'm sure this is not the intended behavior, so is it possible I'm missing a less-specific conf setting or something? All of this works fine of course, when press-enabled=true. Everything compiles great. I am so sorry for the trouble. For what it's worth, I love this module and apart from these less-support issues, it's been a joy to use.

@dirkmc
Copy link
Owner

dirkmc commented Aug 10, 2012

hmm yeah you're right that's not how it should work. I guess if press is disabled the less files should still get compiled to CSS, but not compressed. I'd probably add a configuration option in there so that you can still serve raw less if you are trying to debug something.

It should be fairly easy to change. I already took some of the code from the Less module, I'd just need to copy a couple of the methods from this class:
https://github.com/lunatech-labs/play-module-less/blob/master/src/play/modules/less/Plugin.java

Unfortunately I had to take my laptop in to get it repaired this morning (I'm in an internet cafe) so I won't get a chance to look at it until next week. I'll let you know when it's fixed.

@iamaracinghorse
Copy link

That is totally awesome. Thanks so much for the attention on this!

@dirkmc
Copy link
Owner

dirkmc commented Aug 14, 2012

Still waiting to get my laptop back, hopefully I'll get to this later in the week..

@dirkmc
Copy link
Owner

dirkmc commented Aug 17, 2012

Fixed. Please try version 1.0.36 and let me know how it goes.

@iamaracinghorse
Copy link

Hey, @dirkmc! Sorry the delayed response on this. I've updated to 1.0.36 and removed the LESS module dependency and everything is working like a charm! Thanks again for you attention on this. Total lifesaver.

@dirkmc
Copy link
Owner

dirkmc commented Aug 23, 2012

That's great, I'm glad to hear it's finally working!
Thanks for your help tracking down the bug
Dirk

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

3 participants