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.

Sign up for GitHub

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

Jump to bottom

v4.0.0 #3573

Merged
merged 21 commits into from Dec 18, 2020
Merged

v4.0.0 #3573

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
58c89ee
Remove math=strict-legacy, set default=parens-division
matthew-dean Jul 29, 2020
3d555a6
Support for comma-less color functions
matthew-dean Jul 29, 2020
67ce103
Require parens after mixin call
matthew-dean Jul 29, 2020
93a2ac0
Output invalid JS functions in CSS
matthew-dean Aug 2, 2020
cf86f24
Escape parens and revert pass-through function errors
matthew-dean Sep 1, 2020
fe223b6
Rerun lebab on original ES5
matthew-dean Dec 5, 2020
af37b05
Restore this context for parse/render
matthew-dean Dec 5, 2020
ed73409
Version bump
matthew-dean Dec 5, 2020
fe9ae94
Updated dist files
matthew-dean Dec 5, 2020
92d46f7
Updated dist files
matthew-dean Dec 5, 2020
9a340cd
v4.0.0
matthew-dean Dec 5, 2020
70e242c
Remove ieCompat option
matthew-dean Dec 5, 2020
d6cc422
Add dist files
matthew-dean Dec 5, 2020
b204901
Remove non-performant class pattern
matthew-dean Dec 5, 2020
7573daf
Remove unworking browser benchmark
matthew-dean Dec 6, 2020
3b4a385
Fix linting issues
matthew-dean Dec 9, 2020
0a691a7
Merge branch 'master' into next
matthew-dean Dec 12, 2020
4304557
Update lock file
matthew-dean Dec 12, 2020
e0edcb7
Add isdefined() function
matthew-dean Dec 12, 2020
1c26c28
Merge branch 'master' into next
matthew-dean Dec 18, 2020
8fa48a9
Get Less version from package.json
matthew-dean Dec 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions packages/less/bin/lessc
View file
Open in desktop
Expand Up @@ -554,16 +554,27 @@ function processPluginQueue() {

if (checkArgFunc(arg, match[2])) {
if (checkBooleanArg(match[2])) {
options.math = Constants.Math.STRICT_LEGACY;
options.math = Constants.Math.PARENS;
}
}

break;

case 'm':
case 'math':
if (checkArgFunc(arg, match[2])) {
options.math = match[2];
var m = match[2];
if (checkArgFunc(arg, m)) {
if (m === 'always') {
console.warn('--math=always is deprecated and will be removed in the future.');
Copy link

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Airblader

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 /)

Copy link
Member Author

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!)

Copy link

@Airblader Airblader Jan 25, 2021 •
edited

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!

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link

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!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Airblader

Thanks again for your replies, much appreciated!

You're welcome! If you have Twitter, please pass this along -> https://twitter.com/LessToCSS/status/1353815168581390338

Copy link

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.

options.math = Constants.Math.ALWAYS;
} else if (m === 'parens-division') {
options.math = Constants.Math.PARENS_DIVISION;
} else if (m === 'parens' || m === 'strict') {
options.math = Constants.Math.PARENS;
} else if (m === 'strict-legacy') {
console.warn('--math=strict-legacy has been removed. Defaulting to --math=strict');
options.math = Constants.Math.PARENS;
}
}

break;
Expand Down
2 changes: 1 addition & 1 deletion packages/less/src/less/constants.js
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@ export const Math = {
ALWAYS: 0,
PARENS_DIVISION: 1,
PARENS: 2,
STRICT_LEGACY: 3
// removed - STRICT_LEGACY: 3
};

export const RewriteUrls = {
Expand Down
44 changes: 22 additions & 22 deletions packages/less/src/less/default-options.js
View file
Open in desktop
Expand Up @@ -7,60 +7,60 @@ export default () => ({
depends: false,

/* (DEPRECATED) Compress using less built-in compression.
* This does an okay job but does not utilise all the tricks of
* dedicated css compression. */
* This does an okay job but does not utilise all the tricks of
* dedicated css compression. */
compress: false,

/* Runs the less parser and just reports errors without any output. */
lint: false,

/* Sets available include paths.
* If the file in an @import rule does not exist at that exact location,
* less will look for it at the location(s) passed to this option.
* You might use this for instance to specify a path to a library which
* you want to be referenced simply and relatively in the less files. */
* If the file in an @import rule does not exist at that exact location,
* less will look for it at the location(s) passed to this option.
* You might use this for instance to specify a path to a library which
* you want to be referenced simply and relatively in the less files. */
paths: [],

/* color output in the terminal */
color: true,

/* The strictImports controls whether the compiler will allow an @import inside of either
* @media blocks or (a later addition) other selector blocks.
* See: https://github.com/less/less.js/issues/656 */
* @media blocks or (a later addition) other selector blocks.
* See: https://github.com/less/less.js/issues/656 */
strictImports: false,

/* Allow Imports from Insecure HTTPS Hosts */
insecure: false,

/* Allows you to add a path to every generated import and url in your css.
* This does not affect less import statements that are processed, just ones
* that are left in the output css. */
* This does not affect less import statements that are processed, just ones
* that are left in the output css. */
rootpath: '',

/* By default URLs are kept as-is, so if you import a file in a sub-directory
* that references an image, exactly the same URL will be output in the css.
* This option allows you to re-write URL's in imported files so that the
* URL is always relative to the base imported file */
* that references an image, exactly the same URL will be output in the css.
* This option allows you to re-write URL's in imported files so that the
* URL is always relative to the base imported file */
rewriteUrls: false,

/* How to process math
* 0 always - eagerly try to solve all operations
* 1 parens-division - require parens for division "/"
* 2 parens | strict - require parens for all operations
* 3 strict-legacy - legacy strict behavior (super-strict)
*/
math: 0,
* 0 always - eagerly try to solve all operations
* 1 parens-division - require parens for division "/"
* 2 parens | strict - require parens for all operations
* 3 strict-legacy - legacy strict behavior (super-strict)
*/
math: 1,

/* Without this option, less attempts to guess at the output unit when it does maths. */
strictUnits: false,

/* Effectively the declaration is put at the top of your base Less file,
* meaning it can be used but it also can be overridden if this variable
* is defined in the file. */
* meaning it can be used but it also can be overridden if this variable
* is defined in the file. */
globalVars: null,

/* As opposed to the global variable option, this puts the declaration at the
* end of your base file, meaning it will override anything defined in your Less file. */
* end of your base file, meaning it will override anything defined in your Less file. */
modifyVars: null,

/* This option allows you to specify a argument to go on to every URL. */
Expand Down
40 changes: 17 additions & 23 deletions packages/less/src/less/functions/function-caller.js
View file
Open in desktop
Expand Up @@ -15,36 +15,30 @@ class functionCaller {
}

call(args) {
if (!(Array.isArray(args))) {
args = [args];
}
const evalArgs = this.func.evalArgs;
if (evalArgs !== false) {
args = args.map(a => a.eval(this.context));
}
const commentFilter = item => !(item.type === 'Comment');

// This code is terrible and should be replaced as per this issue...
// https://github.com/less/less.js/issues/2477
if (Array.isArray(args)) {
args = args.filter(item => {
if (item.type === 'Comment') {
return false;
}
return true;
})
.map(item => {
if (item.type === 'Expression') {
const subNodes = item.value.filter(item => {
if (item.type === 'Comment') {
return false;
}
return true;
});
if (subNodes.length === 1) {
return subNodes[0];
} else {
return new Expression(subNodes);
}
args = args
.filter(commentFilter)
.map(item => {
if (item.type === 'Expression') {
const subNodes = item.value.filter(commentFilter);
if (subNodes.length === 1) {
return subNodes[0];
} else {
return new Expression(subNodes);
}
return item;
});
}
}
return item;
});

if (evalArgs === false) {
return this.func(this.context, ...args);
Expand Down
4 changes: 1 addition & 3 deletions packages/less/src/less/tree/expression.js
View file
Open in desktop
Expand Up @@ -23,9 +23,7 @@ class Expression extends Node {
eval(context) {
let returnValue;
const mathOn = context.isMathOn();

const inParenthesis = this.parens &&
(context.math !== MATH.STRICT_LEGACY || !this.parensInOp);
const inParenthesis = this.parens;

let doubleParen = false;
if (inParenthesis) {
Expand Down
6 changes: 3 additions & 3 deletions packages/less/src/less/utils.js
View file
Open in desktop
Expand Up @@ -59,7 +59,7 @@ export function copyOptions(obj1, obj2) {
}
const opts = defaults(obj1, obj2);
if (opts.strictMath) {
opts.math = Constants.Math.STRICT_LEGACY;
opts.math = Constants.Math.PARENS;
}
// Back compat with changed relativeUrls option
if (opts.relativeUrls) {
Expand All @@ -77,8 +77,8 @@ export function copyOptions(obj1, obj2) {
case 'parens':
opts.math = Constants.Math.PARENS;
break;
case 'strict-legacy':
opts.math = Constants.Math.STRICT_LEGACY;
default:
opts.math = Constants.Math.PARENS;
}
}
if (typeof opts.rewriteUrls === 'string') {
Expand Down
17 changes: 9 additions & 8 deletions packages/less/test/index.js
View file
Open in desktop
Expand Up @@ -13,15 +13,15 @@ var testMap = [
javascriptEnabled: true
}, '_main/'],
[{}, 'namespacing/'],
[{
math: 'strict-legacy'
}, 'math/strict-legacy/'],
[{
math: 'parens'
}, 'math/strict/'],
[{
math: 'parens-division'
}, 'math/parens-division/'],
[{
math: 'always'
}, 'math/always/'],
// Use legacy strictMath: true here to demonstrate it still works
[{strictMath: true, strictUnits: true, javascriptEnabled: true}, '../errors/eval/',
lessTester.testErrors, null],
Expand All @@ -39,8 +39,10 @@ var testMap = [
// TODO: Change this to rewriteUrls: false once the relativeUrls option is removed
[{math: 'strict', relativeUrls: false, rootpath: 'folder (1)/'}, 'static-urls/'],
[{math: 'strict', compress: true}, 'compression/'],
[{math: 0, strictUnits: true}, 'strict-units/'],
[{}, 'legacy/'],

[{math: 0, strictUnits: true}, 'units/strict/'],
[{math: 0, strictUnits: false}, 'units/no-strict/'],

[{math: 'strict', strictUnits: true, sourceMap: true, globalVars: true }, 'sourcemaps/',
lessTester.testSourcemap, null, null,
function(filename, type, baseFolder) {
Expand Down Expand Up @@ -74,16 +76,15 @@ var testMap = [
[{plugin: 'test/plugins/preprocess/'}, 'preProcessorPlugin/'],
[{plugin: 'test/plugins/visitor/'}, 'visitorPlugin/'],
[{plugin: 'test/plugins/filemanager/'}, 'filemanagerPlugin/'],
[{}, 'no-strict-math/'],
[{}, '3rd-party/'],
[{math: 0}, '3rd-party/'],
[{ processImports: false }, 'process-imports/']
];
testMap.forEach(function(args) {
lessTester.runTestSet.apply(lessTester, args)
});
lessTester.testSyncronous({syncImport: true}, '_main/import');
lessTester.testSyncronous({syncImport: true}, '_main/plugin');
lessTester.testSyncronous({syncImport: true}, 'math/strict-legacy/css');
lessTester.testSyncronous({syncImport: true}, 'math/strict/css');
lessTester.testNoOptions();
lessTester.testJSImport();
lessTester.finished();
0 ...data/css/no-strict-math/mixins-guards.css → ...st-data/css/math/always/mixins-guards.css
View file
Open in desktop
File renamed without changes.
0 ...a/css/no-strict-math/no-sm-operations.css → ...data/css/math/always/no-sm-operations.css
View file
Open in desktop
File renamed without changes.
95 changes: 0 additions & 95 deletions packages/test-data/css/math/strict-legacy/css.css
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions packages/test-data/css/math/strict-legacy/media-math.css
View file
Open in desktop

This file was deleted.