-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
v4.0.0 #3573
v4.0.0 #3573
Conversation
# Conflicts: # dist/less.js # dist/less.min.js # dist/less.min.js.map # packages/less/dist/less.js # packages/less/dist/less.min.js # packages/less/dist/less.min.js.map # packages/less/package.json # packages/less/src/less/index.js
var m = match[2]; | ||
if (checkArgFunc(arg, m)) { | ||
if (m === 'always') { | ||
console.warn('--math=always is deprecated and will be removed in the future.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthew-dean This deprecation is not mentioned in the docs.
It's also a really huge breaking change for existing apps. The fact that less doesn't ship some kind of migration tooling for that is a bit of a let down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A migration tool would be non-trivial. Fair enough that the deprecation isn't mentioned. But since you can
a) wrap division in parens to indicate you want math performed,
b) not use v4 if you don't want the default, how is this deprecation notice a huge breaking change?
The "will be removed" language should probably be removed. There's no guarantee it will be removed; it's just not a recommended compiler setting because it makes the /
token ambiguous in your stylesheets, and it often doesn't do what you'd expect. (See lots and lots of issues filed over time around the behavior of /
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The long and short is that /
is a valid separator in CSS values, so parsing this as a division operation is problematic (even if division is not performed!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A migration tool would be non-trivial.
Neither is fixing potentially hundreds of divisions in your app, especially since this doesn't fail at build time, so you're just lost and have to search for all occurrences yourself.
not use v4 if you don't want the default
I assume(d) v3 won't be maintained in parallel to v4, so updating is, sooner or later, a requirement. Maybe I'm wrong about that. In our specific case the upgrade to less v4 came from the UI library and we would've had to manually keep it downgraded.
it's just not a recommended compiler setting because it makes the / token ambiguous in your stylesheets
In a few, rare properties only, though. Virtually all others don't suffer from this problem (assuming there isn't some blatant reason I'm missing — there might be). Admittedly I don't know how less works internally, so I don't know whether less could evaluate divisions without parentheses for all the properties where no ambiguity exists.
Either way, for our project it's "too late", we already did the update. I just wanted to leave feedback that this caused quite a bit of headache, also there not being a changelog (that I could find) with the breaking changes didn't make it easy to discover at first what the issue even is; we eventually found it scouring the docs. Thanks for taking the time to answer!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way, for our project it's "too late", we already did the update. I just wanted to leave feedback that this caused quite a bit of headache, also there not being a changelog (that I could find) with the breaking changes didn't make it easy to discover at first what the issue even is; we eventually found it scouring the docs. Thanks for taking the time to answer!
I apologize, Less has been low on contributions and I don't have a great deal of time to devote, so I switched to an auto-changelog because otherwise the complaint was that there was no changelog for some releases.
I'm actively seeking people who can do more repo / package maintenance for Less, especially around releasing / releasing documentation. It would be great to get a team of people who actively rely on Less.
Neither is fixing potentially hundreds of divisions in your app
😬 I can see how that could be tedious / unexpected. This has long been discussed in issue threads here but it's easy to forget people don't follow those!
Specifically about the line about removal of that option, I don't think that's a likely outcome now at this point. Less will have parens-division
by default, but always
will likely always remain an option for the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Airblader Please message me on Gitter or Twitter if you or your team would like to take a more active role in maintaining Less.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Less has been low on contributions
Yeah, trust me, I know what that is like... :-)
This has long been discussed in issue threads here but it's easy to forget people don't follow those!
I will say that at least the change came in a major version, which is of course a fair thing to do. A migration tool would've been awesome, but open source also means the community would have to step up. I gave my feedback here completely ignorant to how active the project is.
That said, I'm already busy enough with my open source work so unfortunately I won't be able to take that up myself.
Specifically about the line about removal of that option, I don't think that's a likely outcome now at this point.
(Unfortunately, Angluar CLI still has no way of customizing the less options, but that's obviously not a less issue at all)
Thanks again for your replies, much appreciated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your replies, much appreciated!
You're welcome! If you have Twitter, please pass this along -> https://twitter.com/LessToCSS/status/1353815168581390338
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course! I'll also share it on our company to see if anyone is interested in helping out.
No description provided.