Skip to content

Commit ddec00d

Browse files
zaquestzaquest
committed
Fix less#3576 import redirects. Replace native-request with needle.
1 parent bfad962 commit ddec00d

File tree

7 files changed

+129
-40
lines changed

7 files changed

+129
-40
lines changed

‎package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/less/package-lock.json

Lines changed: 54 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/less/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"image-size": "~0.5.0",
5151
"make-dir": "^2.1.0",
5252
"mime": "^1.4.1",
53-
"native-request": "^1.0.5",
53+
"needle": "^2.5.2",
5454
"source-map": "~0.6.0"
5555
},
5656
"devDependencies": {
@@ -82,6 +82,7 @@
8282
"mocha": "^6.2.1",
8383
"mocha-headless-chrome": "^2.0.3",
8484
"mocha-teamcity-reporter": "^3.0.0",
85+
"nock": "^13.0.5",
8586
"npm-run-all": "^4.1.5",
8687
"performance-now": "^0.2.0",
8788
"phin": "^2.2.3",

‎packages/less/src/less-node/url-file-manager.js

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ UrlFileManager.prototype = Object.assign(new AbstractFileManager(), {
1313
loadFile(filename, currentDirectory, options, environment) {
1414
return new Promise((fulfill, reject) => {
1515
if (request === undefined) {
16-
try { request = require('native-request'); }
16+
try { request = require('needle'); }
1717
catch (e) { request = null; }
1818
}
1919
if (!request) {
@@ -22,23 +22,27 @@ UrlFileManager.prototype = Object.assign(new AbstractFileManager(), {
2222
}
2323

2424
let urlStr = isUrlRe.test( filename ) ? filename : url.resolve(currentDirectory, filename);
25-
25+
2626
/** native-request currently has a bug */
2727
const hackUrlStr = urlStr.indexOf('?') === -1 ? urlStr + '?' : urlStr
2828

29-
request.get(hackUrlStr, (error, body, status) => {
30-
if (status === 404) {
31-
reject({ type: 'File', message: `resource '${urlStr}' was not found\n` });
29+
request.get(hackUrlStr, { follow_max: 5 }, (err, resp, body) => {
30+
if (err || resp.statusCode >= 400) {
31+
const message = resp.statusCode === 404
32+
? `resource '${urlStr}' was not found\n`
33+
: `resource '${urlStr}' gave this Error:\n ${err || resp.statusMessage || resp.statusCode}\n`;
34+
reject({ type: 'File', message });
3235
return;
3336
}
34-
if (error) {
35-
reject({ type: 'File', message: `resource '${urlStr}' gave this Error:\n ${error}\n` });
37+
if (resp.statusCode >= 300) {
38+
reject({ type: 'File', message: `resource '${urlStr}' caused too many redirects` });
3639
return;
3740
}
41+
body = body.toString('utf8');
3842
if (!body) {
39-
logger.warn(`Warning: Empty body (HTTP ${status}) returned by "${urlStr}"`);
43+
logger.warn(`Warning: Empty body (HTTP ${resp.statusCode}) returned by "${urlStr}"`);
4044
}
41-
fulfill({ contents: body, filename: urlStr });
45+
fulfill({ contents: body || '', filename: urlStr });
4246
});
4347
});
4448
}

‎packages/less/test/index.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
var lessTest = require('./less-test'),
22
lessTester = lessTest(),
33
path = require('path'),
4-
stylize = require('../lib/less-node/lessc-helper').stylize;
4+
stylize = require('../lib/less-node/lessc-helper').stylize,
5+
nock = require('nock');
56

67
console.log('\n' + stylize('Less', 'underline') + '\n');
78

@@ -39,7 +40,7 @@ var testMap = [
3940
// TODO: Change this to rewriteUrls: false once the relativeUrls option is removed
4041
[{math: 'strict', relativeUrls: false, rootpath: 'folder (1)/'}, 'static-urls/'],
4142
[{math: 'strict', compress: true}, 'compression/'],
42-
43+
4344
[{math: 0, strictUnits: true}, 'units/strict/'],
4445
[{math: 0, strictUnits: false}, 'units/no-strict/'],
4546

@@ -88,3 +89,20 @@ lessTester.testSyncronous({syncImport: true}, 'math/strict/css');
8889
lessTester.testNoOptions();
8990
lessTester.testJSImport();
9091
lessTester.finished();
92+
93+
(() => {
94+
// Create new tester, since tests are not independent and tests
95+
// above modify tester in a way that breaks remote imports.
96+
lessTester = lessTest();
97+
var scope = nock('https://example.com')
98+
.get('/redirect.less').query(true)
99+
.reply(301, null, { location: '/target.less' })
100+
.get('/target.less').query(true)
101+
.reply(200);
102+
lessTester.runTestSet(
103+
{},
104+
'import-redirect/',
105+
lessTester.testImportRedirect(scope)
106+
);
107+
lessTester.finished();
108+
})();

‎packages/less/test/less-test.js

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
/* jshint latedef: nofunc */
22

3+
var logger = require('../lib/less/logger').default;
4+
5+
var isVerbose = process.env.npm_config_loglevel !== 'concise';
6+
logger.addListener({
7+
info(msg) {
8+
if (isVerbose) {
9+
process.stdout.write(msg + '\n');
10+
}
11+
},
12+
warn(msg) {
13+
process.stdout.write(msg + '\n');
14+
},
15+
erro(msg) {
16+
process.stdout.write(msg + '\n');
17+
}
18+
});
19+
20+
321
module.exports = function() {
422
var path = require('path'),
523
fs = require('fs'),
@@ -14,8 +32,6 @@ module.exports = function() {
1432
var oneTestOnly = process.argv[2],
1533
isFinished = false;
1634

17-
var isVerbose = process.env.npm_config_loglevel !== 'concise';
18-
1935
var testFolder = path.dirname(require.resolve('@less/test-data'));
2036
var lessFolder = path.join(testFolder, 'less');
2137

@@ -27,20 +43,6 @@ module.exports = function() {
2743
}
2844
}
2945

30-
less.logger.addListener({
31-
info: function(msg) {
32-
if (isVerbose) {
33-
process.stdout.write(msg + '\n');
34-
}
35-
},
36-
warn: function(msg) {
37-
process.stdout.write(msg + '\n');
38-
},
39-
error: function(msg) {
40-
process.stdout.write(msg + '\n');
41-
}
42-
});
43-
4446
var queueList = [],
4547
queueRunning = false;
4648
function queue(func) {
@@ -519,6 +521,23 @@ module.exports = function() {
519521
ok(stylize('OK\n', 'green'));
520522
}
521523

524+
function testImportRedirect(nockScope) {
525+
return (name, err, css, doReplacements, sourcemap, baseFolder) => {
526+
process.stdout.write('- ' + path.join(baseFolder, name) + ': ');
527+
if (err) {
528+
fail('FAIL: ' + (err && err.message));
529+
return;
530+
}
531+
const expected = 'h1 {\n color: red;\n}\n';
532+
if (css !== expected) {
533+
difference('FAIL', expected, css);
534+
return;
535+
}
536+
nockScope.done();
537+
ok('OK');
538+
};
539+
}
540+
522541
return {
523542
runTestSet: runTestSet,
524543
runTestSetNormalOnly: runTestSetNormalOnly,
@@ -527,6 +546,7 @@ module.exports = function() {
527546
testSourcemap: testSourcemap,
528547
testSourcemapWithoutUrlAnnotation: testSourcemapWithoutUrlAnnotation,
529548
testImports: testImports,
549+
testImportRedirect: testImportRedirect,
530550
testEmptySourcemap: testEmptySourcemap,
531551
testNoOptions: testNoOptions,
532552
testJSImport: testJSImport,

‎packages/test-data/less/import-redirect/import-redirect.less

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@import "https://example.com/redirect.less";
2+
3+
h1 { color: red; }

0 commit comments

Comments
 (0)