Skip to content
This repository has been archived by the owner on Dec 21, 2024. It is now read-only.

Commit

Permalink
Improve error handling
Browse files Browse the repository at this point in the history
I added some "integration tests" where we use electron-hot-loader to
require some react components.

This has allowed to test that errors thrown by the compiler are as
simple as possible.

Indeed, when a component requires another component, the errors
will be nested and hard to read.
  • Loading branch information
geowarin committed Mar 10, 2016
2 parents 8e41eeb + 11e6f28 commit 505370a
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 28 deletions.
10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
"description": "Hot reloading for React components in electron without babel nor webpack",
"main": "src/index.js",
"scripts": {
"test": "mocha",
"test:unit": "mocha",
"test:integ": "mocha --opts test-integration/mocha.opts",
"test": "npm-run-all test:*",
"prepublish": "npm run test"
},
"repository": {
Expand All @@ -30,7 +32,11 @@
"watch-glob": "0.1.3"
},
"devDependencies": {
"describe-with-dom": "^1.0.0",
"expect": "^1.14.0",
"mocha": "^2.4.5"
"mocha": "^2.4.5",
"npm-run-all": "^1.5.2",
"react": "^0.14.7",
"react-dom": "^0.14.7"
}
}
37 changes: 14 additions & 23 deletions src/jsxTransform.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,36 +37,27 @@ function transform(filename, source, options) {

function install(options) {
if (installed) {
return
return;
}

var fs = require('fs');
var Module = require('module');
var _require = Module.prototype.require;

options = options || {};
if (!options.hasOwnProperty('sourceMapInline')) {
options.sourceMapInline = true;
}

Module._extensions[options.extension || '.jsx'] = function (module, filename) {
if (!options.hasOwnProperty('sourceMapInline')) {
options.sourceMapInline = true
}

var content = fs.readFileSync(filename, 'utf8');
require.extensions[options.extension || '.jsx'] = function loadJsx(module, filename) {
var content = require('fs').readFileSync(filename, 'utf8');
try {
var instrumented = transform(filename, content, options);
module._compile(instrumented, filename)
module._compile(instrumented, filename);
} catch (e) {
console.error("Error compiling " + filename, e);
console.error(e.stack);
if (e.originalError) {
throw e.originalError;
}
e.message = 'Error compiling ' + filename + ': ' + e.message;
e.originalError = e;
throw e;
}
};

Module.prototype.require = function (filename) {
if ('!' === filename.slice(-1)) {
filename = filename.slice(0, -1)
}
return _require.call(this, filename)
};

installed = true
}
}
7 changes: 4 additions & 3 deletions src/transforms/react-jsx-visitors.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,15 @@ function visitReactTag(traverse, object, path, state) {
// JSXMemberExpressions which look like Foo.Bar.Baz. This also handles
// JSXIdentifiers that aren't fallback tags.
// GWA: monkey patch
if (state.g.opts.doNotInstrument !== true) {
const requirePath = state.g.requireNodesMap && state.g.requireNodesMap[nameObject.name];
if (state.g.opts.doNotInstrument !== true && requirePath) {
utils.append('__electronHot__.register(', state);
}
utils.move(nameObject.range[0], state);
utils.catchup(nameObject.range[1], state);
// GWA: monkey patch
if (state.g.opts.doNotInstrument !== true) {
utils.append(", require.resolve('" + state.g.requireNodesMap[nameObject.name] + "'))", state);
if (state.g.opts.doNotInstrument !== true && requirePath) {
utils.append(", require.resolve('" + requirePath + "'))", state);
}
}

Expand Down
43 changes: 43 additions & 0 deletions test-integration/load.spec.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
"use strict";

const expect = require('expect');
const describeWithDom = require('describe-with-dom');

const React = require('react');
const ReactDOM = require('react-dom');

describeWithDom('loadJsx', () => {

it('should load a JSX file', () => {
const App = require('./views/App.jsx');
const element = document.createElement('div');
document.body.appendChild(element);
ReactDOM.render(<App/>, element);

var window = document.defaultView;
expect(window.document.documentElement.outerHTML).toContain('Hello');
});

it('should throw when a component contains an error', () => {
const exceptionMessage = getExceptionMessage(() => require('./views/ErrorComponent.jsx'));
expect(exceptionMessage)
.toMatch(/^Error compiling [\w\/\-\\]+?ErrorComponent\.jsx: Parse Error: Line 10: Unexpected token }/)
});

it('should throw a simple error when a component contains a component which contains an error', () => {
const exceptionMessage = getExceptionMessage(() => require('./views/AppUsingErrorComponent.jsx'));
expect(exceptionMessage)
.toMatch(/^Error compiling [\w\/\-\\]+?ErrorComponent\.jsx: Parse Error: Line 10: Unexpected token }/)
});
});

function getExceptionMessage(executor) {
let message;
try {
executor();
} catch (e) {
message = e.message;
}
expect(message).toExist('Expected method to throw');
return message;
}
5 changes: 5 additions & 0 deletions test-integration/mocha.opts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-u bdd
--recursive ./test-integration/**/*spec.jsx
--full-trace
--reporter spec
--require ./compiler
10 changes: 10 additions & 0 deletions test-integration/views/App.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
"use strict";

const React = require('react');

module.exports = class App extends React.Component {

render() {
return (<div>Hello</div>)
}
};
11 changes: 11 additions & 0 deletions test-integration/views/AppUsingErrorComponent.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"use strict";

const React = require('react');
const Component = require('./ErrorComponent.jsx');

module.exports = class App extends React.Component {

render() {
return <Component />
}
};
11 changes: 11 additions & 0 deletions test-integration/views/ErrorComponent.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"use strict";

const React = require('react');

// This component intentionnaly contains a parse error
module.exports = class App extends React.Component {

render() {
return (<div>Hello<div>)
}
};

0 comments on commit 505370a

Please sign in to comment.