-
Notifications
You must be signed in to change notification settings - Fork 39
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
Updgrade to less 2.0 #53
Comments
@richdougherty @jroper Less 2.5.0 was released on April 3. What's needed to be done for |
Here's what needs doing:
|
I've actually done some work on this. It's not a trivial upgrade, requires a fix to the less-node webjar, I've got it working on node, but for trireme, there's a few dependencies (eg, es6 promises) that we don't have webjars for yet, and I've been too busy to follow up. |
@richdougherty good plan! @jroper would you be so kind to share your branch? I guess I'll be able to look at the upgrade over the weekend. |
@yatskevich https://github.com/jroper/sbt-less/tree/bintray-upgrades Note that I originally was just going to reconfigure it to publish to bintray (we're moving all plugins to bintray), and I decided to do a version refresh while I was at it, that's when I started down the rabbit hole of the upgrade. The version of the less webjar that it refers to is here: |
Short summary:
|
@yatskevich: Awesome, thanks. I think @jroper has access to the webjars repo, so he may be able to merge your PR. |
We can update the scripted test to account for the less.js issue, just put a comment on it to explain why it's testing for the numbers its testing for, so that if/when less is fixed, and we upgrade, we know why the test fails. Alternatively, we can adjust the error reporting in sbt-less, since we know how many global vars/modify vars were added, so we can adjust the line numbers accordingly. Again, if you decide to do that, put a comment linking to the issue that explains why, so that if it's fixed in future, we know what to do. |
Yep, I was thinking about these options too. I'd better keep sbt-less error reporting as close as possible to lessc reporting to avoid further confusion and adjust scripted test. |
Final version (PR #62) contains workaround for the lessc issue (if globalVars or modifyVars are not provided they are not passed to lessc). Original test (error-output) passes. |
@yatskevich First thank you for your great work. Second, I tried to compile and use your snapshot version but it does not work for me. Are you sure its really resolved and working? Are all dependencies up-to-date? When I compile my project with this updated version of sbt-less plugin it throws the following error:
Would you look at this error and explained it to me, please? Is there any mistake in my configuration or so? With the current stable version the project works (but compiles wrong as I need Less v2.5). Thanks! |
@KarelCemus Could you please share your configuration? |
In plugins.sbt I have these (and couple more but they should be unrelated):
and in build.sbt
And that's all related to Less. Would you like to see the whole project conf or is this enough? |
@KarelCemus config looks good to me. I guess the error is caused by contents of one of your less files. Have you tried compiling it with pure lessc? |
It doesn't compile even when the files are completely empty. And I think the files are good, I received them from our coder who uses Grunt so they should compile with pure lessc fine. This is complete plugins.sbt in case of some interaction.
|
@yatskevich I tried to figure out the problem and give you the smallest project to reproduce it and I found first real issue. When there is file to compile and it is empty or contains only comments, it fails with the error I posted earlier. I confirmed that. Now I have to find out how this relates to my project configuration. Can you reproduce it yourself or should I give you the whole project? It's tiny. |
@yatskevich OK and this bug relates to my project. I have 4 main files and one of them was empty. So that's it, thank you for your assistance. |
Just curious. Is this empty file issue a regression given the PR? |
It could be a regression in less itself - maybe see what happens when you compile an empty less file with less. |
I verified that less 2.5 compiles empty file without an error. Will add corresponding test to the plugin's test suite. |
The issue is confirmed. It happens because less returns the following compilation result: { css: '', map: undefined, imports: [] } So the error I've tested original less behavior and here is the result. Would you expect the same result from sbt-less or a new issue should be submitted for less.js? |
Seems like a new issue to me. |
They have this issue reported and closed already - less/less.js#2430, but my testing on 2.4.0 and 2.5.0 shows that it's not actually fixed. |
Less 2.0.0 was released on Nov 11: https://github.com/less/less.js/blob/master/CHANGELOG.md#200
The text was updated successfully, but these errors were encountered: