Skip to content

Commit

Permalink
Fix file path issues (#833)
Browse files Browse the repository at this point in the history
* Create tests for compileClient file writing.

* Apply fixes for various path issues in compileClient.  Remove console.log instrumentation.  Update gulpfile and test spec.

* Remove out of date comments regarding tests not being present.

* Restore callerDependency to it's original state.  Create a mock callerDependency and add mockery to handle properly requiring the mock instead of the original callerDependency.

* Removed unnecessary getSpecGlob() function.
  • Loading branch information
drewpc authored and gigabo committed Jan 27, 2017
1 parent 10cf9b9 commit 2e8d9c1
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 14 deletions.
7 changes: 5 additions & 2 deletions packages/react-server-cli/gulpfile.babel.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import eslint from "gulp-eslint";
import gulp from "gulp";
import babel from "gulp-babel";
import jasmine from "gulp-jasmine";
import logging from "react-server-gulp-module-tagger";

gulp.task("default", () => {
Expand All @@ -23,8 +24,10 @@ gulp.task("eslint", [], () => {
.pipe(eslint.failAfterError());
});

// there are no tests for this project :(
gulp.task("test", ["eslint"]);
gulp.task("test", ["default", "eslint"], () => {
return gulp.src("target/__tests__/**/*[Ss]pec.js")
.pipe(jasmine({}));
});

gulp.task("watch", () => {
gulp.watch("src/*.js", ['default']);
Expand Down
3 changes: 3 additions & 0 deletions packages/react-server-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
"gulp": "^3.9.1",
"gulp-babel": "^6.1.2",
"gulp-eslint": "^3.0.1",
"gulp-jasmine": "^2.4.2",
"memory-stream": "0.0.3",
"mockery": "^2.0.0",
"nsp": "^2.6.2",
"react-hot-loader": "^1.3.1",
"react-server-gulp-module-tagger": "^0.4.10",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import fs from "fs";
const MemoryStream = require('memory-stream');
import path from "path";
import mockery from "mockery";

describe("compileClient", () => {
let mockFs,
writeWebpackCompatibleRoutesFile;

beforeAll(() => {
mockery.registerSubstitute('./callerDependency', './callerDependency-Mock');
mockery.enable({
useCleanCache: true,
warnOnUnregistered: false,
});

writeWebpackCompatibleRoutesFile = require("../../compileClient").writeWebpackCompatibleRoutesFile;
});

afterAll(() => {
mockery.disable();
});

describe("writes client routes file for Webpack", () => {
const pathStringTests = [
{
title: "apostrophes",
path: "PathWith'InIt.js",
},
{
title: "double quotes",
path: 'PathWith"InIt.js',
},
{
title: "windows style",
path: 'c:\\Path\\With\\InIt.js',
},
{
title: "spaces",
path: 'Path With Spaces.js',
},
];

beforeEach(() => {
mockFs = new MemoryStream();
});

afterEach(() => {
mockFs = null;
});

pathStringTests.map((test) => {
it("handles file paths with " + test.title, (finishTest) => {
spyOn(fs, 'writeFileSync').and.callFake((path, data) => {
mockFs.write(data);
});

const filePath = test.path;
const routes = {
"routes": {
"Homepage": {
"path": "/",
"page": filePath,
},
},
};

writeWebpackCompatibleRoutesFile(routes, ".", path.normalize("."), null, true, null);

const coreMiddlewareStringified = JSON.stringify(require.resolve("react-server-core-middleware"));
const filePathStringified = JSON.stringify(filePath);

// These four strings are important when using multiple platforms or strings with weird characters in them.
// If we're going to output something to a file and then import that file later, we'd better be darn sure
// it's all correctly formatted! Apostrophes, quotes, and windows-style file path characters \ vs / are the worst!
const filePathRegexStrings = [
"var coreJsMiddleware = require(" + coreMiddlewareStringified + ").coreJsMiddleware;",
"var coreCssMiddleware = require(" + coreMiddlewareStringified + ").coreCssMiddleware;",
"require.ensure(" + filePathStringified + ", function() {",
"cb(unwrapEs6Module(require(" + filePathStringified + ")));",
];

const fileData = mockFs.toString();
filePathRegexStrings.map((regex) => {
expect(fileData).toMatch(regex.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&"));
});

finishTest();
});
});
});
});
8 changes: 8 additions & 0 deletions packages/react-server-cli/src/callerDependency-Mock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import lookup from "look-up";
import path from "path";

export default function callerDependency(dep) {
// TODO: We should grab stuff based on what the routes file would get out
// of `require.resolve(dep)`. Using `process.cwd()` instead for now.
return lookup("packages/" + dep, {cwd: path.resolve(process.cwd() + '/..')});
}
1 change: 1 addition & 0 deletions packages/react-server-cli/src/callerDependency.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import lookup from "look-up";

// NOTE: if this function changes, make sure it also changes in the 'callerDependency-Mock.js' file as well.
export default function callerDependency(dep) {
// TODO: We should grab stuff based on what the routes file would get out
// of `require.resolve(dep)`. Using `process.cwd()` instead for now.
Expand Down
20 changes: 10 additions & 10 deletions packages/react-server-cli/src/compileClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,18 @@ function packageCodeForBrowser(entrypoints, outputDir, outputUrl, hot, minify, l
}

// writes out a routes file that can be used at runtime.
function writeWebpackCompatibleRoutesFile(routes, routesDir, workingDirAbsolute, staticUrl, isClient, manifest) {
export function writeWebpackCompatibleRoutesFile(routes, routesDir, workingDirAbsolute, staticUrl, isClient, manifest) {
let routesOutput = [];

const coreMiddleware = require.resolve("react-server-core-middleware");
const coreMiddleware = JSON.stringify(require.resolve("react-server-core-middleware"));
const existingMiddleware = routes.middleware ? routes.middleware.map((middlewareRelativePath) => {
return `unwrapEs6Module(require("${path.relative(workingDirAbsolute, path.resolve(routesDir, middlewareRelativePath))}"))`
return `unwrapEs6Module(require(${JSON.stringify(path.relative(workingDirAbsolute, path.resolve(routesDir, middlewareRelativePath)))}))`
}) : [];
routesOutput.push(`
var manifest = ${manifest ? JSON.stringify(manifest) : "undefined"};
function unwrapEs6Module(module) { return module.__esModule ? module.default : module }
var coreJsMiddleware = require('${coreMiddleware}').coreJsMiddleware;
var coreCssMiddleware = require('${coreMiddleware}').coreCssMiddleware;
var coreJsMiddleware = require(${coreMiddleware}).coreJsMiddleware;
var coreCssMiddleware = require(${coreMiddleware}).coreCssMiddleware;
module.exports = {
middleware:[
coreJsMiddleware(${JSON.stringify(staticUrl)}, manifest),
Expand Down Expand Up @@ -323,22 +323,22 @@ module.exports = {
page: {`);
for (let format of Object.keys(formats)) {
const formatModule = formats[format];
var relativePathToPage = path.relative(workingDirAbsolute, path.resolve(routesDir, formatModule));
var relativePathToPage = JSON.stringify(path.relative(workingDirAbsolute, path.resolve(routesDir, formatModule)));
routesOutput.push(`
${format}: function() {
return {
done: function(cb) {`);
if (isClient) {
routesOutput.push(`
require.ensure("${relativePathToPage}", function() {
cb(unwrapEs6Module(require("${relativePathToPage}")));
require.ensure(${relativePathToPage}, function() {
cb(unwrapEs6Module(require(${relativePathToPage})));
});`);
} else {
routesOutput.push(`
try {
cb(unwrapEs6Module(require("${relativePathToPage}")));
cb(unwrapEs6Module(require(${relativePathToPage})));
} catch (e) {
console.error('Failed to load page at "${relativePathToPage}"', e.stack);
console.error('Failed to load page at ${relativePathToPage}', e.stack);
}`);
}
routesOutput.push(`
Expand Down
2 changes: 0 additions & 2 deletions packages/react-server/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,4 @@ gulp.task("eslint", [], function() {
.pipe(eslint.failAfterError());
});

// todo: where should tests go?

// todo: add clean

0 comments on commit 2e8d9c1

Please sign in to comment.