Feature/rewrite urls (#3248)

* Rename relativeUrls option to rewriteUrls * Refactor contexts.js - Rename isPathRelative to pathRequiresRewrite - Add special handling for rewriteUrls=relative - Add rewritePath for path rewriting - Ensure that explicit relative paths stay explicit relative - Remove code style inconsistencies * Add missing tests for url rewriting * Rename rewrite-urls option value to explicit-relative * Add missing tests for url rewriting * Refactor rewrite urls options - Rename explicit-relative to local - Add rewrite-urls argument validation - Replace unknown CSS property background-url with background-image Based on discussion #3041 * Re-add tests for deprecated relative-urls option * Improve tests - Remove redundant tests - Add rootpath-rewrite-urls-all test - Add rootpath-rewrite-urls-local test * Fix typo in unknown argument warning * Revert old tests to deprecated relativeUrls option again * Added more CSS Grid tests * All tests passing * Merge branch 'master' into feature/rewrite-urls (#3282) * WIP - Added strictMath: 'division' option * Removes `less-rhino` (broken for a long time) - Fixes #3241 * Expressions require a delimiter of some kind in mixin args * Removes "double paren" issue for boolean / if function * WIP each() re-implementation * Allows plain conditions without parentheses * Added each() and tests * Added tests calling mixins from each() * Fixes #1880 - Adds two new math modes and deprecates strictMath * Allows named args for detached ruleset (anonymous mixin) * Makes sure this doesn't regress needing parens around mixin guard conditions * Remove javascriptEnabled from browser options check, add to defaults * Workaround weird Jasmine conversion of < to HTML entity * Removes remaining Rhino cruft * Remove rhino files from bower * Bump Jasmine version * Added .() example to each * v3.6.0 * Update CHANGELOG.md * add explicit nested anonymous mixin test * add explicit nested anonymous mixin test result * derp: align test with expected output * v3.7.0 (#3279) * Update CHANGELOG.md * Rewrite the rewriteUrls feature to use same pattern as strictMath-to-math * Update console warning for --relative-urls
less · Jul 18, 2018 · 2d43c0b · 2d43c0b
1 parent b0228c9
commit 2d43c0b
Show file tree

Hide file tree

Showing 43 changed files with 466 additions and 63 deletions.
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -70,14 +70,18 @@ module.exports = function (grunt) {
 
     var sauceJobs = {};
 
-    var browserTests = [   'filemanager-plugin',
+
+    var browserTests = [
+        'filemanager-plugin',
         'visitor-plugin',
         'global-vars',
         'modify-vars',
         'production',
         'rootpath-relative',
+        'rootpath-rewrite-urls',
         'rootpath',
         'relative-urls',
+        'rewrite-urls',
         'browser',
         'no-js-errors',
         'legacy'
@@ -362,6 +366,14 @@ module.exports = function (grunt) {
                     outfile: 'tmp/browser/test-runner-relative-urls.html'
                 }
             },
+            rewriteUrls: {
+                src: ['test/browser/less/rewrite-urls/*.less'],
+                options: {
+                    helpers: 'test/browser/runner-rewrite-urls-options.js',
+                    specs: 'test/browser/runner-rewrite-urls-spec.js',
+                    outfile: 'tmp/browser/test-runner-rewrite-urls.html'
+                }
+            },
             rootpath: {
                 src: ['test/browser/less/rootpath/*.less'],
                 options: {
@@ -378,6 +390,14 @@ module.exports = function (grunt) {
                     outfile: 'tmp/browser/test-runner-rootpath-relative.html'
                 }
             },
+            rootpathRewriteUrls: {
+                src: ['test/browser/less/rootpath-rewrite-urls/*.less'],
+                options: {
+                    helpers: 'test/browser/runner-rootpath-rewrite-urls-options.js',
+                    specs: 'test/browser/runner-rootpath-rewrite-urls-spec.js',
+                    outfile: 'tmp/browser/test-runner-rootpath-rewrite-urls.html'
+                }
+            },
             production: {
                 src: ['test/browser/less/production/*.less'],
                 options: {

diff --git a/bin/lessc b/bin/lessc
@@ -4,7 +4,7 @@ var path = require('path'),
     fs = require('../lib/less-node/fs'),
     os = require('os'),
     utils = require('../lib/less/utils'),
-    MATH = require('../lib/less/math-constants'),
+    Constants = require('../lib/less/constants'),
     errno,
     mkdirp;
 
@@ -476,16 +476,35 @@ function processPluginQueue() {
                     options.rootpath = match[2].replace(/\\/g, '/');
                 }
                 break;
-            case 'ru':
             case 'relative-urls':
-                options.relativeUrls = true;
+                console.warn('The --relative-urls option has been deprecated. Use --rewrite-urls=all.');
+                options.rewriteUrls = Constants.RewriteUrls.ALL;
+                break;
+            case 'ru':
+            case 'rewrite-urls':
+                var m = match[2];
+                if (m) {
+                    if (m === 'local') {
+                        options.rewriteUrls = Constants.RewriteUrls.LOCAL;
+                    } else if (m === 'off') {
+                        options.rewriteUrls = Constants.RewriteUrls.OFF;
+                    } else if (m === 'all') {
+                        options.rewriteUrls = Constants.RewriteUrls.ALL;
+                    } else {
+                        console.error('Unknown rewrite-urls argument ' + m);
+                        continueProcessing = false;
+                        process.exitCode = 1;
+                    }
+                } else {
+                    options.rewriteUrls = Constants.RewriteUrls.ALL;
+                }
                 break;
             case 'sm':
             case 'strict-math':
                 console.warn('The --strict-math option has been deprecated. Use --math=strict.');
                 if (checkArgFunc(arg, match[2])) {
                     if (checkBooleanArg(match[2])) {
-                        options.math = MATH.STRICT_LEGACY;
+                        options.math = Constants.Math.STRICT_LEGACY;
                     }
                 }
                 break;

diff --git a/lib/less-browser/add-default-options.js b/lib/less-browser/add-default-options.js
@@ -42,4 +42,8 @@ module.exports = function(window, options) {
     if (options.onReady === undefined) {
         options.onReady = true;
     }
+
+    if (options.relativeUrls) {
+        options.rewriteUrls = 'all';
+    }
 };
diff --git a/lib/less-browser/index.js b/lib/less-browser/index.js
@@ -93,7 +93,8 @@ module.exports = function(window, options) {
                 currentDirectory: fileManager.getPath(path),
                 filename: path,
                 rootFilename: path,
-                relativeUrls: instanceOptions.relativeUrls};
+                rewriteUrls: instanceOptions.rewriteUrls
+            };
 
             newFileInfo.entryPath = newFileInfo.currentDirectory;
             newFileInfo.rootpath = instanceOptions.rootpath || newFileInfo.currentDirectory;

diff --git a/lib/less-node/image-size.js b/lib/less-node/image-size.js
@@ -6,7 +6,7 @@ module.exports = function(environment) {
     function imageSize(functionContext, filePathNode) {
         var filePath = filePathNode.value;
         var currentFileInfo = functionContext.currentFileInfo;
-        var currentDirectory = currentFileInfo.relativeUrls ?
+        var currentDirectory = currentFileInfo.rewriteUrls ?
         currentFileInfo.currentDirectory : currentFileInfo.entryPath;
 
         var fragmentStart = filePath.indexOf('#');

diff --git a/lib/less-node/lessc-helper.js b/lib/less-node/lessc-helper.js
@@ -47,7 +47,8 @@ var lessc_helper = {
         console.log('                               in generated CSS file.');
         console.log('  -rp, --rootpath=URL          Sets rootpath for url rewriting in relative imports and urls');
         console.log('                               Works with or without the relative-urls option.');
-        console.log('  -ru, --relative-urls         Re-writes relative urls to the base less file.');
+        console.log('  -ru=, --rewrite-urls=        Rewrites URLs to make them relative to the base less file.');
+        console.log('    all|local|off              \'all\' rewrites all URLs, \'local\' just those starting with a \'.\'');
         console.log('');
         console.log('  -m=, --math=');
         console.log('     always                    Less will eagerly perform math operations always.');

diff --git a/lib/less/constants.js b/lib/less/constants.js
@@ -0,0 +1,13 @@
+module.exports = {
+    Math: {
+        ALWAYS: 0,
+        PARENS_DIVISION: 1,
+        PARENS: 2,
+        STRICT_LEGACY: 3
+    },
+    RewriteUrls: {
+        OFF: 0,
+        LOCAL: 1,
+        ALL: 2
+    }
+};
diff --git a/lib/less/contexts.js b/lib/less/contexts.js
@@ -1,6 +1,6 @@
 var contexts = {};
 module.exports = contexts;
-var MATH = require('./math-constants');
+var Constants = require('./constants');
 
 var copyFromOriginal = function copyFromOriginal(original, destination, propertiesToCopy) {
     if (!original) { return; }
@@ -18,7 +18,7 @@ var copyFromOriginal = function copyFromOriginal(original, destination, properti
 var parseCopyProperties = [
     // options
     'paths',            // option - unmodified - paths to search for imports on
-    'relativeUrls',     // option - whether to adjust URL's to be relative
+    'rewriteUrls',      // option - whether to adjust URL's to be relative
     'rootpath',         // option - rootpath to append to URL's
     'strictImports',    // option -
     'insecure',         // option - whether to allow imports from insecure ssl hosts
@@ -51,7 +51,8 @@ var evalCopyProperties = [
     'urlArgs',           // whether to add args into url tokens
     'javascriptEnabled', // option - whether Inline JavaScript is enabled. if undefined, defaults to false
     'pluginManager',     // Used as the plugin manager for the session
-    'importantScope'     // used to bubble up !important statements
+    'importantScope',    // used to bubble up !important statements
+    'rewriteUrls'        // option - whether to adjust URL's to be relative
 ];
 
 contexts.Eval = function(options, frames) {
@@ -95,26 +96,45 @@ contexts.Eval.prototype.isMathOn = function (op) {
     if (!this.mathOn) {
         return false;
     }
-    if (op === '/' && this.math !== MATH.ALWAYS && (!this.parensStack || !this.parensStack.length)) {
+    if (op === '/' && this.math !== Constants.Math.ALWAYS && (!this.parensStack || !this.parensStack.length)) {
         return false;
     }
-    if (this.math > MATH.PARENS_DIVISION) {
+    if (this.math > Constants.Math.PARENS_DIVISION) {
         return this.parensStack && this.parensStack.length;
     }
     return true;
 };
 
-contexts.Eval.prototype.isPathRelative = function (path) {
-    return !/^(?:[a-z-]+:|\/|#)/i.test(path);
+contexts.Eval.prototype.pathRequiresRewrite = function (path) {
+    var isRelative = this.rewriteUrls === Constants.RewriteUrls.LOCAL ? isPathLocalRelative : isPathRelative;
+
+    return isRelative(path);
 };
 
-contexts.Eval.prototype.normalizePath = function( path ) {
+contexts.Eval.prototype.rewritePath = function (path, rootpath) {
+    var newPath;
+
+    rootpath = rootpath || '';
+    newPath = this.normalizePath(rootpath + path);
+
+    // If a path was explicit relative and the rootpath was not an absolute path
+    // we must ensure that the new path is also explicit relative.
+    if (isPathLocalRelative(path) &&
+        isPathRelative(rootpath) &&
+        isPathLocalRelative(newPath) === false) {
+        newPath = './' + newPath;
+    }
+
+    return newPath;
+};
+
+contexts.Eval.prototype.normalizePath = function (path) {
     var
         segments = path.split('/').reverse(),
         segment;
 
     path = [];
-    while (segments.length !== 0 ) {
+    while (segments.length !== 0) {
         segment = segments.pop();
         switch ( segment ) {
             case '.':
@@ -127,12 +147,20 @@ contexts.Eval.prototype.normalizePath = function( path ) {
                 }
                 break;
             default:
-                path.push( segment );
+                path.push(segment);
                 break;
         }
     }
 
     return path.join('/');
 };
 
+function isPathRelative(path) {
+    return !/^(?:[a-z-]+:|\/|#)/i.test(path);
+}
+
+function isPathLocalRelative(path) {
+    return path.charAt(0) === '.';
+}
+
 // todo - do the same for the toCSS ?
diff --git a/lib/less/default-options.js b/lib/less/default-options.js
@@ -42,7 +42,7 @@ module.exports = function() {
          * 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 */
-        relativeUrls: false,
+        rewriteUrls: false,
 
         /* Compatibility with IE8. Used for limiting data-uri length */
         ieCompat: false,  // true until 3.0

diff --git a/lib/less/functions/data-uri.js b/lib/less/functions/data-uri.js
@@ -18,7 +18,7 @@ module.exports = function(environment) {
         var mimetype = mimetypeNode && mimetypeNode.value;
         var filePath = filePathNode.value;
         var currentFileInfo = this.currentFileInfo;
-        var currentDirectory = currentFileInfo.relativeUrls ?
+        var currentDirectory = currentFileInfo.rewriteUrls ?
             currentFileInfo.currentDirectory : currentFileInfo.entryPath;
 
         var fragmentStart = filePath.indexOf('#');

diff --git a/lib/less/import-manager.js b/lib/less/import-manager.js
@@ -8,7 +8,7 @@ var contexts = require('./contexts'),
 module.exports = function(environment) {
 
     // FileInfo = {
-    //  'relativeUrls' - option - whether to adjust URL's to be relative
+    //  'rewriteUrls' - option - whether to adjust URL's to be relative
     //  'filename' - full resolved filename of current file
     //  'rootpath' - path to append to normal URLs for this node
     //  'currentDirectory' - path to the current file, absolute
@@ -65,7 +65,7 @@ module.exports = function(environment) {
         };
 
         var newFileInfo = {
-            relativeUrls: this.context.relativeUrls,
+            rewriteUrls: this.context.rewriteUrls,
             entryPath: currentFileInfo.entryPath,
             rootpath: currentFileInfo.rootpath,
             rootFilename: currentFileInfo.rootFilename
@@ -92,7 +92,7 @@ module.exports = function(environment) {
             // - If path of imported file is '../mixins.less' and rootpath is 'less/',
             //   then rootpath should become 'less/../'
             newFileInfo.currentDirectory = fileManager.getPath(resolvedFilename);
-            if (newFileInfo.relativeUrls) {
+            if (newFileInfo.rewriteUrls) {
                 newFileInfo.rootpath = fileManager.join(
                     (importManager.context.rootpath || ''),
                     fileManager.pathDiff(newFileInfo.currentDirectory, newFileInfo.entryPath));

diff --git a/lib/less/math-constants.js b/lib/less/math-constants.js
diff --git a/lib/less/parse.js b/lib/less/parse.js
@@ -46,7 +46,7 @@ module.exports = function(environment, ParseTree, ImportManager) {
                 var entryPath = filename.replace(/[^\/\\]*$/, '');
                 rootFileInfo = {
                     filename: filename,
-                    relativeUrls: context.relativeUrls,
+                    rewriteUrls: context.rewriteUrls,
                     rootpath: context.rootpath || '',
                     currentDirectory: entryPath,
                     entryPath: entryPath,

diff --git a/lib/less/tree/declaration.js b/lib/less/tree/declaration.js
@@ -2,7 +2,7 @@ var Node = require('./node'),
     Value = require('./value'),
     Keyword = require('./keyword'),
     Anonymous = require('./anonymous'),
-    MATH = require('../math-constants');
+    MATH = require('../constants').Math;
 
 var Declaration = function (name, value, important, merge, index, currentFileInfo, inline, variable) {
     this.name = name;

diff --git a/lib/less/tree/expression.js b/lib/less/tree/expression.js
@@ -2,7 +2,7 @@ var Node = require('./node'),
     Paren = require('./paren'),
     Comment = require('./comment'),
     Dimension = require('./dimension'),
-    MATH = require('../math-constants');
+    MATH = require('../constants').Math;
 
 var Expression = function (value, noSpacing) {
     this.value = value;

diff --git a/lib/less/tree/import.js b/lib/less/tree/import.js
@@ -97,17 +97,18 @@ Import.prototype.evalForImport = function (context) {
 };
 Import.prototype.evalPath = function (context) {
     var path = this.path.eval(context);
-    var rootpath = this._fileInfo && this._fileInfo.rootpath;
+    var fileInfo = this._fileInfo;
 
     if (!(path instanceof URL)) {
-        if (rootpath) {
-            var pathValue = path.value;
-            // Add the base path if the import is relative
-            if (pathValue && context.isPathRelative(pathValue)) {
-                path.value = rootpath + pathValue;
-            }
+        // Add the rootpath if the URL requires a rewrite
+        var pathValue = path.value;
+        if (fileInfo &&
+            pathValue &&
+            context.pathRequiresRewrite(pathValue)) {
+            path.value = context.rewritePath(pathValue, fileInfo.rootpath);
+        } else {
+            path.value = context.normalizePath(path.value);
         }
-        path.value = context.normalizePath(path.value);
     }
 
     return path;

diff --git a/lib/less/tree/operation.js b/lib/less/tree/operation.js
@@ -1,7 +1,7 @@
 var Node = require('./node'),
     Color = require('./color'),
     Dimension = require('./dimension'),
-    MATH = require('../math-constants');
+    MATH = require('../constants').Math;
 
 var Operation = function (op, operands, isSpaced) {
     this.op = op.trim();