From 22c560c70a9c637d93a29e47a6d558b04e1ccefd Mon Sep 17 00:00:00 2001 From: xzyfer Date: Fri, 25 Mar 2016 19:11:13 +1100 Subject: [PATCH] Remove use of global process.sass This replaces the global `process.sass` with proper module exports. Unfortunately `process.sass`, although undocumented, is available to other modules. This patch keeps the existing public API available until we bump the major. --- bin/node-sass | 3 +- lib/extensions.js | 71 ++++++++++++++++++++++++++-------------------- lib/index.js | 25 +++------------- scripts/build.js | 13 ++++----- scripts/install.js | 16 +++++------ test/cli.js | 4 +-- test/lowlevel.js | 25 ++++++++-------- test/runtime.js | 62 ++++++++++++++++++++-------------------- 8 files changed, 104 insertions(+), 115 deletions(-) diff --git a/bin/node-sass b/bin/node-sass index 7a2bf93d5..fdc9bacd9 100755 --- a/bin/node-sass +++ b/bin/node-sass @@ -8,6 +8,7 @@ var Emitter = require('events').EventEmitter, util = require('util'), path = require('path'), glob = require('glob'), + sass = require('../lib'), render = require('../lib/render'), stdin = require('get-stdin'), fs = require('fs'); @@ -18,7 +19,7 @@ var Emitter = require('events').EventEmitter, var cli = meow({ pkg: '../package.json', - version: process.sass.versionInfo, + version: sass.info, help: [ 'Usage:', ' node-sass [options] ', diff --git a/lib/extensions.js b/lib/extensions.js index 687d71844..fd549021d 100644 --- a/lib/extensions.js +++ b/lib/extensions.js @@ -2,27 +2,28 @@ * node-sass: lib/extensions.js */ -var flags = {}, +var eol = require('os').EOL, fs = require('fs'), pkg = require('../package.json'), path = require('path'); /** - * Collect Arguments + * Get the value of a CLI argument * + * @param {String} name * @param {Array} args * @api private */ -function collectArguments(args) { - for (var i = 0; i < args.length; i += 2) { - if (args[i].lastIndexOf('--', 0) !== 0) { - --i; - continue; - } +function getArgument(name, args) { + var flags = args || process.argv.slice(2), + index = flags.lastIndexOf(name); - flags[args[i]] = args[i + 1]; + if (index === -1 || index + 1 >= flags.length) { + return null; } + + return flags[index + 1]; } /** @@ -33,14 +34,14 @@ function collectArguments(args) { * return it as is, otherwise make default binary * name: {platform}-{arch}-{v8 version}.node * - * @api private + * @api public */ function getBinaryName() { var binaryName; - if (flags['--sass-binary-name']) { - binaryName = flags['--sass-binary-name']; + if (getArgument('--sass-binary-name')) { + binaryName = getArgument('--sass-binary-name'); } else if (process.env.SASS_BINARY_NAME) { binaryName = process.env.SASS_BINARY_NAME; } else if (process.env.npm_config_sass_binary_name) { @@ -63,7 +64,7 @@ function getBinaryName() { * * The default URL can be overriden using * the environment variable SASS_BINARY_SITE, -* .npmrc variable sass_binary_site or + * .npmrc variable sass_binary_site or * or a command line option --sass-binary-site: * * node scripts/install.js --sass-binary-site http://example.com/ @@ -81,27 +82,19 @@ function getBinaryName() { * v3.0.0/freebsd-x64-42_binding.node * ... etc. for all supported versions and platforms * - * @api private + * @api public */ function getBinaryUrl() { - var site = flags['--sass-binary-site'] || + var site = getArgument('--sass-binary-site') || process.env.SASS_BINARY_SITE || process.env.npm_config_sass_binary_site || (pkg.nodeSassConfig && pkg.nodeSassConfig.binarySite) || 'https://github.com/sass/node-sass/releases/download'; - return [site, 'v' + pkg.version, sass.binaryName].join('/'); + return [site, 'v' + pkg.version, getBinaryName()].join('/'); } - -collectArguments(process.argv.slice(2)); - -var sass = process.sass = {}; - -sass.binaryName = getBinaryName(); -sass.binaryUrl = getBinaryUrl(); - /** * Get binary path. * If environment variable SASS_BINARY_PATH, @@ -114,14 +107,14 @@ sass.binaryUrl = getBinaryUrl(); * returning. * * @param {Boolean} throwIfNotExists - * @api private + * @api public */ -sass.getBinaryPath = function(throwIfNotExists) { +function getBinaryPath(throwIfNotExists) { var binaryPath; - if (flags['--sass-binary-path']) { - binaryPath = flags['--sass-binary-path']; + if (getArgument('--sass-binary-path')) { + binaryPath = getArgument('--sass-binary-path'); } else if (process.env.SASS_BINARY_PATH) { binaryPath = process.env.SASS_BINARY_PATH; } else if (process.env.npm_config_sass_binary_path) { @@ -129,7 +122,7 @@ sass.getBinaryPath = function(throwIfNotExists) { } else if (pkg.nodeSassConfig && pkg.nodeSassConfig.binaryPath) { binaryPath = pkg.nodeSassConfig.binaryPath; } else { - binaryPath = path.join(__dirname, '..', 'vendor', sass.binaryName.replace(/_/, '/')); + binaryPath = path.join(__dirname, '..', 'vendor', getBinaryName().replace(/_/, '/')); } if (!fs.existsSync(binaryPath) && throwIfNotExists) { @@ -141,6 +134,22 @@ sass.getBinaryPath = function(throwIfNotExists) { } return binaryPath; -}; +} + +/** + * Get Sass version information + * + * @api public + */ + +function getVersionInfo(binding) { + return [ + ['node-sass', pkg.version, '(Wrapper)', '[JavaScript]'].join('\t'), + ['libsass ', binding.libsassVersion(), '(Sass Compiler)', '[C/C++]'].join('\t'), + ].join(eol); +} -sass.binaryPath = sass.getBinaryPath(); +module.exports.getBinaryUrl = getBinaryUrl; +module.exports.getBinaryName = getBinaryName; +module.exports.getBinaryPath = getBinaryPath; +module.exports.getVersionInfo = getVersionInfo; diff --git a/lib/index.js b/lib/index.js index b176d07b3..73b7cc9c1 100644 --- a/lib/index.js +++ b/lib/index.js @@ -2,31 +2,15 @@ * node-sass: lib/index.js */ -var eol = require('os').EOL, - path = require('path'), +var path = require('path'), util = require('util'), - pkg = require('../package.json'); - -require('./extensions'); + sass = require('./extensions'); /** * Require binding */ -var binding = require(process.sass.getBinaryPath(true)); - -/** - * Get Sass version information - * - * @api private - */ - -function getVersionInfo(binding) { - return [ - ['node-sass', pkg.version, '(Wrapper)', '[JavaScript]'].join('\t'), - ['libsass ', binding.libsassVersion(), '(Sass Compiler)', '[C/C++]'].join('\t'), - ].join(eol); -} +var binding = require(sass.getBinaryPath(true)); /** * Get input file @@ -430,8 +414,7 @@ module.exports.renderSync = function(options) { * @api public */ -process.sass.versionInfo = getVersionInfo(binding); -module.exports.info = process.sass.versionInfo; +module.exports.info = sass.getVersionInfo(binding); /** * Expose sass types diff --git a/scripts/build.js b/scripts/build.js index 65764351e..2147c43e5 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -7,9 +7,8 @@ var eol = require('os').EOL, fs = require('fs'), mkdir = require('mkdirp'), path = require('path'), - spawn = require('cross-spawn-async'); - -require('../lib/extensions'); + spawn = require('cross-spawn-async'), + sass = require('../lib/extensions'); /** * After build @@ -19,7 +18,7 @@ require('../lib/extensions'); */ function afterBuild(options) { - var install = process.sass.binaryPath; + var install = sass.getBinaryPath(); var target = path.join(__dirname, '..', 'build', options.debug ? 'Debug' : process.config.target_defaults.default_configuration, 'binding.node'); @@ -197,13 +196,11 @@ function testBinary(options) { return build(options); } - try { - process.sass.getBinaryPath(true); - } catch (e) { + if (!sass.getBinaryPath()) { return build(options); } - console.log('`', process.sass.binaryPath, '` exists.', eol, 'testing binary.'); + console.log('`', sass.getBinaryPath(), '` exists.', eol, 'testing binary.'); try { require('../').renderSync({ diff --git a/scripts/install.js b/scripts/install.js index c93812b28..421a61686 100644 --- a/scripts/install.js +++ b/scripts/install.js @@ -7,9 +7,8 @@ var fs = require('fs'), mkdir = require('mkdirp'), path = require('path'), got = require('got'), - pkg = require('../package.json'); - -require('../lib/extensions'); + pkg = require('../package.json'), + sass = require('../lib/extensions'); /** * Download file, if succeeds save, if not delete @@ -82,24 +81,23 @@ function applyProxy(options, cb) { */ function checkAndDownloadBinary() { - try { - process.sass.getBinaryPath(true); + if (sass.getBinaryPath()) { return; - } catch (e) { } + } - mkdir(path.dirname(process.sass.binaryPath), function(err) { + mkdir(path.dirname(sass.getBinaryPath()), function(err) { if (err) { console.error(err); return; } - download(process.sass.binaryUrl, process.sass.binaryPath, function(err) { + download(sass.getBinaryUrl(), sass.getBinaryPath(), function(err) { if (err) { console.error(err); return; } - console.log('Binary downloaded and installed at', process.sass.binaryPath); + console.log('Binary downloaded and installed at', sass.binaryPath()); }); }); } diff --git a/test/cli.js b/test/cli.js index d9561268a..3a20ea31b 100644 --- a/test/cli.js +++ b/test/cli.js @@ -11,7 +11,7 @@ var assert = require('assert'), LIBSASS_VERSION = null; describe('cli', function() { - + before(function(done) { var bin = spawn(cli, ['-v']); bin.stdout.setEncoding('utf8'); @@ -480,7 +480,7 @@ describe('cli', function() { '--source-map-embed', '--source-map', 'true' ]); - + bin.stdout.on('data', function(data) { result += data; }); diff --git a/test/lowlevel.js b/test/lowlevel.js index d4383af23..84f9201ce 100644 --- a/test/lowlevel.js +++ b/test/lowlevel.js @@ -1,7 +1,8 @@ process.env.NODESASS_COV ? require('../lib-cov') : require('../lib'); var assert = require('assert'), - binding = require(process.sass.binaryPath); + sass = require('../lib/extensions'), + binding = require(sass.getBinaryPath()); describe('lowlevel', function() { it('fail with options not an object', function(done) { @@ -13,7 +14,7 @@ describe('lowlevel', function() { }); it('data context with options.data not provided', function(done) { - var options = { + var options = { /* data: */ sourceComments: false, file: null, @@ -28,13 +29,13 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderSync(options); - assert(/Data context created without a source string/.test(options.result.error), + assert(/Data context created without a source string/.test(options.result.error), 'Should fail with error message "Data context created without a source string"'); done(); }); it('data context with both options.data and options.file not provided', function(done) { - var options = { + var options = { /* data: */ sourceComments: false, /* file: null, */ @@ -49,13 +50,13 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderSync(options); - assert(/Data context created without a source string/.test(options.result.error), + assert(/Data context created without a source string/.test(options.result.error), 'Should fail with error message "Data context created without a source string"'); done(); }); it('file context with both options.data and options.file not provided', function(done) { - var options = { + var options = { /* data: */ sourceComments: false, /* file: null, */ @@ -70,13 +71,13 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderFileSync(options); - assert(/File context created without an input path/.test(options.result.error), + assert(/File context created without an input path/.test(options.result.error), 'Should fail with error message "File context created without an input path"'); done(); }); it('file context with options.file not provided, options.data given', function(done) { - var options = { + var options = { data: 'div { width: 10px; } ', sourceComments: false, /* file: null, */ @@ -91,7 +92,7 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderFileSync(options); - assert(/File context created without an input path/.test(options.result.error), + assert(/File context created without an input path/.test(options.result.error), 'Should fail with error message "File context created without an input path"'); done(); }); @@ -213,14 +214,14 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderSync(options); - assert(/empty source string/.test(options.result.error), + assert(/empty source string/.test(options.result.error), 'Should fail with error message "Data context created with empty source string"'); done(); }); it('empty file string', function(done) { - var options = { + var options = { sourceComments: false, file: '', outFile: null, @@ -234,7 +235,7 @@ describe('lowlevel', function() { result: { stats: {} } }; binding.renderFileSync(options); - assert(/empty input path/.test(options.result.error), + assert(/empty input path/.test(options.result.error), 'Should fail with error message "File context created with empty input path"'); done(); }); diff --git a/test/runtime.js b/test/runtime.js index 05f746c74..722b7494f 100644 --- a/test/runtime.js +++ b/test/runtime.js @@ -1,11 +1,11 @@ var assert = require('assert'), - fs = require('fs'); + fs = require('fs'), + extensionsPath = process.env.NODESASS_COV + ? require.resolve('../lib-cov/extensions') + : require.resolve('../lib/extensions'); describe('runtime parameters', function() { var packagePath = require.resolve('../package'), - extensionsPath = process.env.NODESASS_COV - ? require.resolve('../lib-cov/extensions') - : require.resolve('../lib/extensions'), // Let's use JSON to fake a deep copy savedArgv = JSON.stringify(process.argv), savedEnv = JSON.stringify(process.env); @@ -21,7 +21,6 @@ describe('runtime parameters', function() { process.argv = JSON.parse(savedArgv); process.env = JSON.parse(savedEnv); require(packagePath); - require(extensionsPath); }); describe('configuration precedence should be respected', function() { @@ -35,29 +34,29 @@ describe('runtime parameters', function() { }); it('command line argument', function() { - require(extensionsPath); - assert.equal(process.sass.binaryName, 'aaa_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryName(), 'aaa_binding.node'); }); it('environment variable', function() { process.argv = []; - require(extensionsPath); - assert.equal(process.sass.binaryName, 'bbb_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryName(), 'bbb_binding.node'); }); it('npm config variable', function() { process.argv = []; process.env.SASS_BINARY_NAME = null; - require(extensionsPath); - assert.equal(process.sass.binaryName, 'ccc_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryName(), 'ccc_binding.node'); }); it('package.json', function() { process.argv = []; process.env.SASS_BINARY_NAME = null; process.env.npm_config_sass_binary_name = null; - require(extensionsPath); - assert.equal(process.sass.binaryName, 'ddd_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryName(), 'ddd_binding.node'); }); }); @@ -70,33 +69,33 @@ describe('runtime parameters', function() { }); it('command line argument', function() { - require(extensionsPath); + var sass = require(extensionsPath); var URL = 'http://aaa.example.com:9999'; - assert.equal(process.sass.binaryUrl.substr(0, URL.length), URL); + assert.equal(sass.getBinaryUrl().substr(0, URL.length), URL); }); it('environment variable', function() { process.argv = []; - require(extensionsPath); + var sass = require(extensionsPath); var URL = 'http://bbb.example.com:8888'; - assert.equal(process.sass.binaryUrl.substr(0, URL.length), URL); + assert.equal(sass.getBinaryUrl().substr(0, URL.length), URL); }); it('npm config variable', function() { process.argv = []; process.env.SASS_BINARY_SITE = null; - require(extensionsPath); + var sass = require(extensionsPath); var URL = 'http://ccc.example.com:7777'; - assert.equal(process.sass.binaryUrl.substr(0, URL.length), URL); + assert.equal(sass.getBinaryUrl().substr(0, URL.length), URL); }); it('package.json', function() { process.argv = []; process.env.SASS_BINARY_SITE = null; process.env.npm_config_sass_binary_site = null; - require(extensionsPath); + var sass = require(extensionsPath); var URL = 'http://ddd.example.com:6666'; - assert.equal(process.sass.binaryUrl.substr(0, URL.length), URL); + assert.equal(sass.getBinaryUrl().substr(0, URL.length), URL); }); }); @@ -109,29 +108,29 @@ describe('runtime parameters', function() { }); it('command line argument', function() { - require(extensionsPath); - assert.equal(process.sass.binaryPath, 'aaa_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryPath(), 'aaa_binding.node'); }); it('environment variable', function() { process.argv = []; - require(extensionsPath); - assert.equal(process.sass.binaryPath, 'bbb_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryPath(), 'bbb_binding.node'); }); it('npm config variable', function() { process.argv = []; process.env.SASS_BINARY_PATH = null; - require(extensionsPath); - assert.equal(process.sass.binaryPath, 'ccc_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryPath(), 'ccc_binding.node'); }); it('package.json', function() { process.argv = []; process.env.SASS_BINARY_PATH = null; process.env.npm_config_sass_binary_path = null; - require(extensionsPath); - assert.equal(process.sass.binaryPath, 'ddd_binding.node'); + var sass = require(extensionsPath); + assert.equal(sass.getBinaryPath(), 'ddd_binding.node'); }); }); @@ -140,12 +139,13 @@ describe('runtime parameters', function() { describe('library detection', function() { it('should throw error when libsass binary is missing.', function() { - var originalBin = process.sass.binaryPath, + var sass = require(extensionsPath), + originalBin = sass.getBinaryPath(), renamedBin = [originalBin, '_moved'].join(''); assert.throws(function() { fs.renameSync(originalBin, renamedBin); - process.sass.getBinaryPath(true); + sass.getBinaryPath(true); }, /The `libsass` binding was not found/); fs.renameSync(renamedBin, originalBin);