From 89f376be6574c44dd39876c2997248d24ee077a0 Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Sun, 15 Jan 2017 18:47:06 +0100 Subject: [PATCH 1/6] Added test for sass compiler --- package.json | 9 +++++++-- test/css/sass.test.js | 21 +++++++++++++++++++++ test/mocha.opts | 4 ++++ test/support.js | 4 ++++ 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 test/css/sass.test.js create mode 100644 test/mocha.opts create mode 100644 test/support.js diff --git a/package.json b/package.json index 97303d4..e15284e 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "scripts": { "doc": "esdoc -c ./esdoc.json", "compile": "git clean -xdf lib && babel -d lib/ src/", - "prepublish": "npm run compile" + "prepublish": "npm run compile", + "test": "./node_modules/.bin/mocha test/**/* -- --harmony" }, "repository": { "type": "git", @@ -52,9 +53,13 @@ "babel-plugin-transform-runtime": "^6.12.0", "babel-preset-es2016-node5": "^1.1.2", "babel-preset-react": "^6.11.1", + "babel-register": "^6.18.0", + "chai": "^3.5.0", + "chai-as-promised": "^6.0.0", "esdoc": "^0.4.8", "esdoc-es7-plugin": "0.0.3", "esdoc-plugin-async-to-sync": "^0.5.0", - "eslint": "^3.3.0" + "eslint": "^3.3.0", + "mocha": "^3.2.0" } } diff --git a/test/css/sass.test.js b/test/css/sass.test.js new file mode 100644 index 0000000..825a59c --- /dev/null +++ b/test/css/sass.test.js @@ -0,0 +1,21 @@ +import SassCompiler from '../../src/css/sass'; + +describe('css/sass', () => { + it('should correctly compile valid sass', function (done) { + this.timeout(5000) + + const sass = 'body, html\n background: red'; + const filename = 'file.sass'; + + const compiler = new SassCompiler(); + compiler.compile(sass, filename) + .then((result) => { + expect(result.mimeType).to.equal('text/css'); + expect(result.code).to.equal('/* line 2, /stdin */\nbody, html {\n background: red; }\n'); + done(); + }) + .catch((err) => { + done(err); + }); + }); +}); diff --git a/test/mocha.opts b/test/mocha.opts new file mode 100644 index 0000000..d23e838 --- /dev/null +++ b/test/mocha.opts @@ -0,0 +1,4 @@ +--require babel-register +--require test/support.js +--reporter spec +--recursive diff --git a/test/support.js b/test/support.js new file mode 100644 index 0000000..e5715d3 --- /dev/null +++ b/test/support.js @@ -0,0 +1,4 @@ +import chai from 'chai' +import chaiAsPromised from 'chai-as-promised' + +global.expect = chai.expect From 0af16872cb4b2defcb0534e92e1afae4b2035859 Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Sun, 15 Jan 2017 20:38:02 +0100 Subject: [PATCH 2/6] Implement `includePaths` option for sass compiler --- src/css/sass.js | 53 +++++++++++++++++++++-- test/css/fixtures/compass/_functions.sass | 0 test/css/fixtures/importable.sass | 2 + test/css/sass.test.js | 35 ++++++++++++--- 4 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 test/css/fixtures/compass/_functions.sass create mode 100644 test/css/fixtures/importable.sass diff --git a/src/css/sass.js b/src/css/sass.js index 29a2784..a7c9daa 100644 --- a/src/css/sass.js +++ b/src/css/sass.js @@ -1,4 +1,5 @@ import path from 'path'; +import fs from 'fs'; import toutSuite from 'toutsuite'; import {CompilerBase} from '../compiler-base'; @@ -48,15 +49,21 @@ export default class SassCompiler extends CompilerBase { paths.unshift('.'); + const { includePaths } = this.compilerOptions + if (includePaths) { + sass.importer(this.buildImporterCallback(includePaths)) + delete this.compilerOptions.includePaths; + } + let opts = Object.assign({}, this.compilerOptions, { indentedSyntax: filePath.match(/\.sass$/i), - sourceMapRoot: filePath, + sourceMapRoot: filePath, }); let result = await new Promise((res,rej) => { sass.compile(sourceCode, opts, (r) => { if (r.status !== 0) { - rej(new Error(r.formatted)); + rej(new Error(r.formatted || r.message)); return; } @@ -93,9 +100,15 @@ export default class SassCompiler extends CompilerBase { paths.unshift('.'); + const { includePaths } = this.compilerOptions + if (includePaths) { + sass.importer(this.buildImporterCallback(includePaths)) + delete this.compilerOptions.includePaths; + } + let opts = Object.assign({}, this.compilerOptions, { indentedSyntax: filePath.match(/\.sass$/i), - sourceMapRoot: filePath, + sourceMapRoot: filePath, }); let result; @@ -114,8 +127,40 @@ export default class SassCompiler extends CompilerBase { }; } + buildImporterCallback (includePaths) { + const resolvedIncludePaths = includePaths.map((includePath) => + path.resolve(process.cwd(), includePath) + ); + + return (function (request, done) { + let file + if (request.file) { + done(); + } else { + // sass.js works in the '/sass/' directory + const cleanedRequestPath = request.resolved.replace(/^\/sass\//, ''); + for (let includePath of includePaths) { + const filePath = path.resolve(includePath, cleanedRequestPath); + const validator = (file) => { + const stat = fs.statSync(file); + if (!stat.isFile()) throw new Error(`${file} is not a file`); + }; + file = sass.findPathVariation(validator, filePath); + if (file) { + const content = fs.readFileSync(file, { encoding: 'utf8' }); + return sass.writeFile(file, content, () => { + done({ path: file }) + }); + } + } + + if (!file) done(); + } + }); + } + getCompilerVersion() { - // NB: There is a bizarre bug in the node module system where this doesn't + // NB: There is a bizarre bug in the node module system where this doesn't // work but only in saveConfiguration tests //return require('@paulcbetts/node-sass/package.json').version; return "4.1.1"; diff --git a/test/css/fixtures/compass/_functions.sass b/test/css/fixtures/compass/_functions.sass new file mode 100644 index 0000000..e69de29 diff --git a/test/css/fixtures/importable.sass b/test/css/fixtures/importable.sass new file mode 100644 index 0000000..7951488 --- /dev/null +++ b/test/css/fixtures/importable.sass @@ -0,0 +1,2 @@ +body, html + background: red diff --git a/test/css/sass.test.js b/test/css/sass.test.js index 825a59c..b8e2a8f 100644 --- a/test/css/sass.test.js +++ b/test/css/sass.test.js @@ -1,21 +1,46 @@ +import path from 'path' import SassCompiler from '../../src/css/sass'; -describe('css/sass', () => { - it('should correctly compile valid sass', function (done) { - this.timeout(5000) - +describe('css/sass', function () { + this.timeout(5000) + it('should correctly compile valid sass', (done) => { const sass = 'body, html\n background: red'; const filename = 'file.sass'; const compiler = new SassCompiler(); + compiler.compilerOptions = { comments: false }; + compiler.compile(sass, filename) .then((result) => { expect(result.mimeType).to.equal('text/css'); - expect(result.code).to.equal('/* line 2, /stdin */\nbody, html {\n background: red; }\n'); + expect(result.code).to.equal('body, html {\n background: red; }\n'); done(); }) .catch((err) => { done(err); }); }); + + describe('when passing includePaths', () => { + it('should correctly resolve import paths', (done) => { + const sass = '@import "importable"'; + const filename = 'file.sass'; + + const compiler = new SassCompiler(); + compiler.compilerOptions = Object.assign({}, compiler.compilerOptions, { + includePaths: ['test/css/fixtures'], + comments: false + }); + + compiler.compile(sass, filename) + .then((result) => { + expect(result.mimeType).to.equal('text/css'); + expect(result.code).to.equal('body, html {\n background: red; }\n'); + done(); + }) + .catch((err) => { + done(err); + }); + }) + }) }); From 6bc4dbe8780fb8aae5b0ab7b1be85ffa9edbdac3 Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Sun, 15 Jan 2017 21:13:07 +0100 Subject: [PATCH 3/6] Fix windows support :( --- src/css/sass.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/css/sass.js b/src/css/sass.js index a7c9daa..42ec38f 100644 --- a/src/css/sass.js +++ b/src/css/sass.js @@ -145,7 +145,36 @@ export default class SassCompiler extends CompilerBase { const stat = fs.statSync(file); if (!stat.isFile()) throw new Error(`${file} is not a file`); }; - file = sass.findPathVariation(validator, filePath); + const variations = sass.getPathVariations(filePath); + file = variations + .map((file) => { + // Unfortunately, there's a bug in sass.js that seems to ignore the different + // path separators across platforms. We need to fix this. + if (process.platform === 'win32' && file[0] === '/') { + file = file.slice(1); + } + + if (file[0] === '_') { + const parts = file.slice(1).split(path.sep); + const dir = parts.slice(0, -1).join(path.sep); + const fileName = parts.reverse()[0]; + file = path.resolve(dir, '_' + fileName); + } + return file + }) + .reduce(function(found, path) { + // Find the first variation that actually exists + if (found) return found; + + try { + const stat = fs.statSync(path); + if (!stat.isFile()) return null; + return path; + } catch(e) { + return null; + } + }, null); + if (file) { const content = fs.readFileSync(file, { encoding: 'utf8' }); return sass.writeFile(file, content, () => { From ee817830170b817eefd8371ac782b12f2a113ed1 Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Mon, 16 Jan 2017 19:52:58 +0100 Subject: [PATCH 4/6] Cleaned up code, add handling of files in same directory --- src/css/sass.js | 70 ++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/src/css/sass.js b/src/css/sass.js index 42ec38f..e26cb20 100644 --- a/src/css/sass.js +++ b/src/css/sass.js @@ -128,10 +128,7 @@ export default class SassCompiler extends CompilerBase { } buildImporterCallback (includePaths) { - const resolvedIncludePaths = includePaths.map((includePath) => - path.resolve(process.cwd(), includePath) - ); - + const self = this; return (function (request, done) { let file if (request.file) { @@ -141,39 +138,11 @@ export default class SassCompiler extends CompilerBase { const cleanedRequestPath = request.resolved.replace(/^\/sass\//, ''); for (let includePath of includePaths) { const filePath = path.resolve(includePath, cleanedRequestPath); - const validator = (file) => { - const stat = fs.statSync(file); - if (!stat.isFile()) throw new Error(`${file} is not a file`); - }; - const variations = sass.getPathVariations(filePath); + let variations = sass.getPathVariations(filePath); + file = variations - .map((file) => { - // Unfortunately, there's a bug in sass.js that seems to ignore the different - // path separators across platforms. We need to fix this. - if (process.platform === 'win32' && file[0] === '/') { - file = file.slice(1); - } - - if (file[0] === '_') { - const parts = file.slice(1).split(path.sep); - const dir = parts.slice(0, -1).join(path.sep); - const fileName = parts.reverse()[0]; - file = path.resolve(dir, '_' + fileName); - } - return file - }) - .reduce(function(found, path) { - // Find the first variation that actually exists - if (found) return found; - - try { - const stat = fs.statSync(path); - if (!stat.isFile()) return null; - return path; - } catch(e) { - return null; - } - }, null); + .map(self.fixWindowsPath.bind(self)) + .reduce(self.importedFileReducer.bind(self), null); if (file) { const content = fs.readFileSync(file, { encoding: 'utf8' }); @@ -188,6 +157,35 @@ export default class SassCompiler extends CompilerBase { }); } + importedFileReducer(found, path) { + // Find the first variation that actually exists + if (found) return found; + + try { + const stat = fs.statSync(path); + if (!stat.isFile()) return null; + return path; + } catch(e) { + return null; + } + } + + fixWindowsPath(file) { + // Unfortunately, there's a bug in sass.js that seems to ignore the different + // path separators across platforms. We need to fix this. + if (process.platform === 'win32' && file[0] === '/') { + file = file.slice(1); + } + + if (file[0] === '_') { + const parts = file.slice(1).split(path.sep); + const dir = parts.slice(0, -1).join(path.sep); + const fileName = parts.reverse()[0]; + file = path.resolve(dir, '_' + fileName); + } + return file + } + getCompilerVersion() { // NB: There is a bizarre bug in the node module system where this doesn't // work but only in saveConfiguration tests From 2e297694c9a4255cb5fd761075ed5f4a05f3cd5d Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Tue, 17 Jan 2017 09:28:02 +0100 Subject: [PATCH 5/6] :fire: Remove test suite --- package.json | 8 +--- test/css/fixtures/compass/_functions.sass | 0 test/css/fixtures/importable.sass | 2 - test/css/sass.test.js | 46 ----------------------- test/mocha.opts | 4 -- test/support.js | 4 -- 6 files changed, 2 insertions(+), 62 deletions(-) delete mode 100644 test/css/fixtures/compass/_functions.sass delete mode 100644 test/css/fixtures/importable.sass delete mode 100644 test/css/sass.test.js delete mode 100644 test/mocha.opts delete mode 100644 test/support.js diff --git a/package.json b/package.json index e15284e..ee0fa59 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,7 @@ "scripts": { "doc": "esdoc -c ./esdoc.json", "compile": "git clean -xdf lib && babel -d lib/ src/", - "prepublish": "npm run compile", - "test": "./node_modules/.bin/mocha test/**/* -- --harmony" + "prepublish": "npm run compile" }, "repository": { "type": "git", @@ -54,12 +53,9 @@ "babel-preset-es2016-node5": "^1.1.2", "babel-preset-react": "^6.11.1", "babel-register": "^6.18.0", - "chai": "^3.5.0", - "chai-as-promised": "^6.0.0", "esdoc": "^0.4.8", "esdoc-es7-plugin": "0.0.3", "esdoc-plugin-async-to-sync": "^0.5.0", - "eslint": "^3.3.0", - "mocha": "^3.2.0" + "eslint": "^3.3.0" } } diff --git a/test/css/fixtures/compass/_functions.sass b/test/css/fixtures/compass/_functions.sass deleted file mode 100644 index e69de29..0000000 diff --git a/test/css/fixtures/importable.sass b/test/css/fixtures/importable.sass deleted file mode 100644 index 7951488..0000000 --- a/test/css/fixtures/importable.sass +++ /dev/null @@ -1,2 +0,0 @@ -body, html - background: red diff --git a/test/css/sass.test.js b/test/css/sass.test.js deleted file mode 100644 index b8e2a8f..0000000 --- a/test/css/sass.test.js +++ /dev/null @@ -1,46 +0,0 @@ -import path from 'path' -import SassCompiler from '../../src/css/sass'; - -describe('css/sass', function () { - this.timeout(5000) - it('should correctly compile valid sass', (done) => { - const sass = 'body, html\n background: red'; - const filename = 'file.sass'; - - const compiler = new SassCompiler(); - compiler.compilerOptions = { comments: false }; - - compiler.compile(sass, filename) - .then((result) => { - expect(result.mimeType).to.equal('text/css'); - expect(result.code).to.equal('body, html {\n background: red; }\n'); - done(); - }) - .catch((err) => { - done(err); - }); - }); - - describe('when passing includePaths', () => { - it('should correctly resolve import paths', (done) => { - const sass = '@import "importable"'; - const filename = 'file.sass'; - - const compiler = new SassCompiler(); - compiler.compilerOptions = Object.assign({}, compiler.compilerOptions, { - includePaths: ['test/css/fixtures'], - comments: false - }); - - compiler.compile(sass, filename) - .then((result) => { - expect(result.mimeType).to.equal('text/css'); - expect(result.code).to.equal('body, html {\n background: red; }\n'); - done(); - }) - .catch((err) => { - done(err); - }); - }) - }) -}); diff --git a/test/mocha.opts b/test/mocha.opts deleted file mode 100644 index d23e838..0000000 --- a/test/mocha.opts +++ /dev/null @@ -1,4 +0,0 @@ ---require babel-register ---require test/support.js ---reporter spec ---recursive diff --git a/test/support.js b/test/support.js deleted file mode 100644 index e5715d3..0000000 --- a/test/support.js +++ /dev/null @@ -1,4 +0,0 @@ -import chai from 'chai' -import chaiAsPromised from 'chai-as-promised' - -global.expect = chai.expect From 8902b1e244eb5d7273df54484bd016ef14a2b2b1 Mon Sep 17 00:00:00 2001 From: Sascha Gehlich Date: Tue, 17 Jan 2017 09:28:26 +0100 Subject: [PATCH 6/6] :art: Code style, comments --- src/css/sass.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/css/sass.js b/src/css/sass.js index e26cb20..2f0337e 100644 --- a/src/css/sass.js +++ b/src/css/sass.js @@ -49,9 +49,9 @@ export default class SassCompiler extends CompilerBase { paths.unshift('.'); - const { includePaths } = this.compilerOptions + const { includePaths } = this.compilerOptions; if (includePaths) { - sass.importer(this.buildImporterCallback(includePaths)) + sass.importer(this.buildImporterCallback(includePaths)); delete this.compilerOptions.includePaths; } @@ -100,9 +100,9 @@ export default class SassCompiler extends CompilerBase { paths.unshift('.'); - const { includePaths } = this.compilerOptions + const { includePaths } = this.compilerOptions; if (includePaths) { - sass.importer(this.buildImporterCallback(includePaths)) + sass.importer(this.buildImporterCallback(includePaths)); delete this.compilerOptions.includePaths; } @@ -130,9 +130,10 @@ export default class SassCompiler extends CompilerBase { buildImporterCallback (includePaths) { const self = this; return (function (request, done) { - let file + let file; if (request.file) { done(); + return; } else { // sass.js works in the '/sass/' directory const cleanedRequestPath = request.resolved.replace(/^\/sass\//, ''); @@ -147,12 +148,16 @@ export default class SassCompiler extends CompilerBase { if (file) { const content = fs.readFileSync(file, { encoding: 'utf8' }); return sass.writeFile(file, content, () => { - done({ path: file }) + done({ path: file }); + return; }); } } - if (!file) done(); + if (!file) { + done(); + return; + } } }); } @@ -172,18 +177,21 @@ export default class SassCompiler extends CompilerBase { fixWindowsPath(file) { // Unfortunately, there's a bug in sass.js that seems to ignore the different - // path separators across platforms. We need to fix this. + // path separators across platforms + + // For some reason, some files have a leading slash that we need to get rid of if (process.platform === 'win32' && file[0] === '/') { file = file.slice(1); } + // Sass.js generates paths such as `_C:\myPath\file.sass` instead of `C:\myPath\_file.sass` if (file[0] === '_') { const parts = file.slice(1).split(path.sep); const dir = parts.slice(0, -1).join(path.sep); const fileName = parts.reverse()[0]; file = path.resolve(dir, '_' + fileName); } - return file + return file; } getCompilerVersion() {