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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow relative paths in scoped packages #12

Merged
merged 19 commits into from
Feb 1, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,9 @@ build/Release
# Dependency directory
# https://www.npmjs.org/doc/misc/npm-faq.html#should-i-check-my-node_modules-folder-into-git
node_modules

# npm debug logs
npm-debug.log*

# Vim swap files
*.sw[pon]
58 changes: 4 additions & 54 deletions lib/importer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,16 @@ var findup = require('findup'),
path = require('path');

var local = require('./local');

function find(dir, file, callback) {
var name;
if (file.split('/')[0][0] === '@') {
name = file.split('/').slice(0, 2).join('/');
} else {
name = file.split('/')[0];
}

var modulePath = './node_modules/' + name + '/package.json';

findup(dir, modulePath, function (err, moduleDir) {
if (err) { return callback(err); }

var root = path.dirname(path.resolve(moduleDir, modulePath));
var location;
// if import is just a module name
if (file === name) {
var json = require(path.resolve(moduleDir, modulePath));
// look for "sass" declaration in package.json
if (json.sass) {
location = json.sass;
// look for "style" declaration in package.json
} else if (json.style) {
location = json.style;
// look for a css/sass/scss file in the "main" declaration in package.json
} else if (/\.(sa|c|sc)ss$/.test(json.main)) {
location = json.main;
// otherwise assume ./styles.scss
} else {
location = './styles';
}
// if a full path is provided
} else {
location = path.join('../', file);
}
callback(null, path.resolve(root, location));
});
}
var resolve = require('./resolver')();

function importer(url, file, done) {
local(url, file, function (err, isLocal) {
if (err || isLocal) {
done({
file: url
});
done({ file: url });
} else {
find(path.dirname(file), url, function (err, location) {
if (err) {
done({
file: url
});
} else {
done({
file: location
});
};
});
done({ file: resolve(url) });
}
})
});
}

module.exports = importer;
19 changes: 19 additions & 0 deletions lib/resolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var Package = require('./resolver/package');
var Import = require('./resolver/import');

module.exports = function resolver(requireFn) {
/* Facilitate testing by allowing requireFn to be specified */
requireFn = (requireFn || require) ;

return function resolve(sassImportPath) {
var _import = new Import(sassImportPath);
var _package = new Package(_import.packageName(), requireFn);

if (_import.isEntrypoint()) {
return _package.resolveEntrypoint();
} else {
return _package.safeResolve(_import.specifiedFilePath());
}
}
}

34 changes: 34 additions & 0 deletions lib/resolver/import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
var path = require('path');

module.exports = Import;

function Import(sassImportPath) {
this.sassImportPath = sassImportPath;
};

Import.prototype.isScoped = function isScoped() {
return this.sassImportPath[0] === '@';
};

Import.prototype.packageName = function packageName() {
if (this.isScoped()) {
return this.sassImportPath.split(path.sep, 2).join(path.sep);
} else {
return this.sassImportPath.split(path.sep, 1)[0];
}
};

Import.prototype.isEntrypoint = function isEntrypoint() {
var safePathSplitPattern = new RegExp(path.sep + '.');
var pathSegmentCount = this.sassImportPath.split(safePathSplitPattern).length;

if (this.isScoped()) {
return pathSegmentCount === 2;
} else {
return pathSegmentCount === 1;
}
};

Import.prototype.specifiedFilePath = function specifiedFilePath() {
return this.sassImportPath.slice(this.packageName().length);
};
46 changes: 46 additions & 0 deletions lib/resolver/package.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
var path = require('path');

module.exports = Package;

function Package(name, requireFn) {
this.requireFn = requireFn;

this.path = path.join.bind(null, name);
};

Package.prototype.json = function packageJSON() {
return this.requireFn(this.path('package.json'));
};

Package.prototype.resolve = function resolve(path) {
return this.requireFn.resolve(this.path(path));
};

Package.prototype.safeResolve = function safeResolve(potentiallyNonExistentPath) {
return path.join(this.dir(), potentiallyNonExistentPath);
};

Package.prototype.dir = function dir() {
return path.dirname(this.resolve('package.json'));
};

Package.prototype.entrypoint = function entrypoint() {
var packageJson = this.json();

if (packageJson.sass) {
return packageJson.sass;
// look for "style" declaration in package.json
} else if (packageJson.style) {
return packageJson.style;
// look for a css/sass/scss file in the "main" declaration in package.json
} else if (/\.(sa|c|sc)ss$/.test(packageJson.main)) {
return packageJson.main;
// otherwise assume ./styles.scss
} else {
return 'styles';
}
};

Package.prototype.resolveEntrypoint = function resolveEntrypoint() {
return this.safeResolve(this.entrypoint());
};
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"main": "index.js",
"bin": "./bin/npm-sass",
"scripts": {
"test": "mocha test/index.js"
"test": "NODE_PATH=$NODE_PATH:\"$(git rev-parse --show-toplevel)/test/fixtures/node_modules\" mocha test/index.js",
"unit-test": "mocha test/units/*.js"
Copy link
Owner

Choose a reason for hiding this comment

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

Would you mind refactoring this a little to rename what is now the test task, and then have test run both of the existing test tasks?

That way we can get better coverage in CI.

},
"repository": {
"type": "git",
Expand All @@ -28,6 +29,7 @@
"devDependencies": {
"chai": "^3.5.0",
"css": "^2.2.1",
"mocha": "^3.0.2"
"mocha": "^3.0.2",
"sinon": "^1.17.6"
}
}
9 changes: 9 additions & 0 deletions test/fixtures/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "npm-sass-test-npm-modules",
"version": "0.0.0",
"description": "sass compilation with npm aware include paths",
"dependencies": {
"@lennym/npm-sass-test-sass": "0.0.0",
"npm-sass-test-sass": "0.0.0"
}
}
22 changes: 6 additions & 16 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,14 @@ const css = require('css');
const root = path.resolve(__dirname, './test-cases');
const cases = fs.readdirSync(root);

cases.forEach((test) => {
let hasPackageJson;

try {
hasPackageJson = require(path.resolve(root, test, './package.json'));
} catch (e) {}
before((done) => {
cp.exec('npm install', { cwd: path.resolve(__dirname, 'fixtures') }, (err) => {
done(err);
});
});

cases.forEach((test) => {
describe(test, () => {
before((done) => {
if (hasPackageJson) {
cp.exec('npm install', { cwd: path.resolve(root, test) }, (err) => {
done(err);
});
} else {
done();
}
});

it('can compile from code without error', (done) => {
sass(path.resolve(root, test, './index.scss'), (err, output) => {
css.parse(output.css.toString());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import "@lennym/npm-sass-test-sass/child-file";
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "npm-sass-test-npm-modules",
"version": "0.0.0",
"description": "sass compilation with npm aware include paths",
"dependencies": {
"@lennym/npm-sass-test-sass": "0.0.0"
}
}
46 changes: 46 additions & 0 deletions test/units/import-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
var Import = require('../../lib/resolver/import');
var assert = require('assert');

describe('Import', function () {
describe('#isScoped', function () {
it('returns true if package is scoped', function () {
var subject1 = new Import('@scoped/package');
var subject2 = new Import('non-scoped-package');

assert(subject1.isScoped());
assert(!subject2.isScoped());
});
});

describe('#packageName', function () {
it('returns the assumed package name', function () {
var subject1 = new Import('@scoped/package/nested-path');
var subject2 = new Import('non-scoped-package/nested-path');

assert.equal(subject1.packageName(), '@scoped/package');
assert.equal(subject2.packageName(), 'non-scoped-package');
});
});

describe('#isEntrypoint', function () {
it('returns true if the import is for the entrypoint', function () {
var subject1 = new Import('@scoped/package/nested-path');
var subject2 = new Import('@scoped/package');
var subject3 = new Import('non-scoped-package/nested-path');
var subject4 = new Import('non-scoped-package');

assert(!subject1.isEntrypoint());
assert(subject2.isEntrypoint());
assert(!subject3.isEntrypoint());
assert(subject4.isEntrypoint());
});
});

describe('#specifiedFilePath', function () {
it('returns the specified nested path', function () {
var subject = new Import('@scoped/package/nested-path1/nested-path2');

assert.equal(subject.specifiedFilePath(), '/nested-path1/nested-path2');
});
});
});