Skip to content

Commit 96f0448

Browse files
author
Ives van Hoorne
committed
Add raw module error detection
1 parent 3433865 commit 96f0448

File tree

8 files changed

+206
-51
lines changed

8 files changed

+206
-51
lines changed

src/app/components/sandbox/Preview/Message.js

Lines changed: 90 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ const Container = styled.div`
2121
box-sizing: border-box;
2222
position: absolute;
2323
background-color: ${({ color }) => {
24-
if (color === 'error') {
25-
return theme.redBackground();
26-
}
27-
return theme.primary();
28-
}};
24+
if (color === 'error') {
25+
return theme.redBackground();
26+
}
27+
return theme.primary();
28+
}};
2929
color: ${() => theme.red()};
3030
height: 100%;
3131
width: 100%;
@@ -35,9 +35,7 @@ const Container = styled.div`
3535
text-align: center;
3636
`;
3737

38-
const Content = styled.div`
39-
width: 70%;
40-
`;
38+
const Content = styled.div`width: 70%;`;
4139

4240
const Icon = styled.div`
4341
margin: 1rem;
@@ -63,10 +61,24 @@ class Message extends React.PureComponent {
6361
getIcon = () => {
6462
const { error, message } = this.props;
6563

66-
if (message) return <Icon><InfoIcon /></Icon>;
67-
if (error.severity === 'warning') return <Icon><WarningIcon /></Icon>;
64+
if (message)
65+
return (
66+
<Icon>
67+
<InfoIcon />
68+
</Icon>
69+
);
70+
if (error.severity === 'warning')
71+
return (
72+
<Icon>
73+
<WarningIcon />
74+
</Icon>
75+
);
6876

69-
return <Icon><ErrorIcon /></Icon>;
77+
return (
78+
<Icon>
79+
<ErrorIcon />
80+
</Icon>
81+
);
7082
};
7183

7284
addDependency = name => async () => {
@@ -80,19 +92,39 @@ class Message extends React.PureComponent {
8092
sandboxActions.setCurrentModule(sandboxId, error.moduleId);
8193
};
8294

95+
renameModuleToJS = () => {
96+
const { error, sandboxId, modules, sandboxActions } = this.props;
97+
const importedModule = modules.find(
98+
m => m.id === error.payload.importedModuleId,
99+
);
100+
101+
sandboxActions.renameModule(
102+
sandboxId,
103+
error.payload.importedModuleId,
104+
`${importedModule.title}.js`,
105+
);
106+
};
107+
83108
getMessage = () => {
84-
const { error, modules, sandboxActions } = this.props;
109+
const { error, modules, sandboxId, sandboxActions } = this.props;
85110

86111
const module = modules.find(m => m.id === error.moduleId) || {};
87112
if (error.type === 'dependency-not-found') {
88113
return (
89114
<div>
90115
Could not find
91-
<b> {"'"}{error.payload.path}{"'"}</b>.
116+
<b>
117+
{' '}{"'"}
118+
{error.payload.path}
119+
{"'"}
120+
</b>.
92121
<div>
93-
Did you add the dependency
94-
{' '}
95-
<b>{"'"}{error.payload.dependency}{"'"}</b>
122+
Did you add the dependency{' '}
123+
<b>
124+
{"'"}
125+
{error.payload.dependency}
126+
{"'"}
127+
</b>
96128
?
97129
</div>
98130
{sandboxActions &&
@@ -113,23 +145,55 @@ class Message extends React.PureComponent {
113145
<div style={{ marginTop: '1rem' }}>
114146
We
115147
{"'"}
116-
re adding support for viewing generated documentation in the near future.
148+
re adding support for viewing generated documentation in the near
149+
future.
117150
{error.payload.react &&
118151
<div>
119-
For now we recommend adding
120-
{' '}
152+
For now we recommend adding{' '}
121153
<Editor readOnly name={error.payload.componentName} />
122154
to view this component.
123155
</div>}
124156
</div>
125157
</div>
126158
);
159+
} else if (error.type === 'raw-react-component-import') {
160+
const importedModule = modules.find(
161+
m => m.id === error.payload.importedModuleId,
162+
);
163+
return (
164+
<div>
165+
It seems like you're trying to render a React component from{' '}
166+
<b>{`'${importedModule.title}'`}</b> in <b>{`'${module.title}'`}</b>,
167+
but <b>{`'${importedModule.title}'`}</b> has no file extension. This
168+
means that it returns a string instead of a component.
169+
<p>
170+
<b>Original error:</b>
171+
<br />
172+
173+
{error.message}
174+
</p>
175+
<div style={{ marginTop: '1rem' }}>
176+
{sandboxActions &&
177+
<div style={{ marginTop: '1rem' }}>
178+
<Button onClick={this.renameModuleToJS} small>
179+
Rename <b>{`'${importedModule.title}'`}</b> to{' '}
180+
<b>{`'${importedModule.title}.js'`}</b>
181+
</Button>
182+
</div>}
183+
</div>
184+
</div>
185+
);
127186
}
128187

129188
return (
130189
<div>
131-
{module && <div>Error in <b>{module.title}</b>:</div>}
132-
<div style={{ marginTop: '1rem' }}>{error.title}: {error.message}</div>
190+
{module &&
191+
<div>
192+
Error in <b>{module.title}</b>:
193+
</div>}
194+
<div style={{ marginTop: '1rem' }}>
195+
{error.title}: {error.message}
196+
</div>
133197

134198
{module &&
135199
sandboxActions &&
@@ -149,7 +213,11 @@ class Message extends React.PureComponent {
149213
<Container color={error && error.severity === 'error' && 'error'}>
150214
<Content>
151215
{this.getIcon()}
152-
{message ? <div>{message}</div> : this.getMessage()}
216+
{message
217+
? <div>
218+
{message}
219+
</div>
220+
: this.getMessage()}
153221
</Content>
154222
</Container>
155223
);

src/app/components/sandbox/Preview/__snapshots__/Message.test.js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ exports[`Message it renders a dependency not found error 1`] = `
66
color="error"
77
>
88
<div
9-
className="sc-EHOje gTncrN"
9+
className="sc-EHOje iBAqQB"
1010
>
1111
<div
1212
className="sc-bZQynM kFeJzc"
@@ -61,7 +61,7 @@ exports[`Message it renders a warning 1`] = `
6161
color={false}
6262
>
6363
<div
64-
className="sc-EHOje gTncrN"
64+
className="sc-EHOje iBAqQB"
6565
>
6666
<div
6767
className="sc-bZQynM kFeJzc"

src/app/store/entities/sandboxes/modules/utils/get-type.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22
import type { Module } from 'common/types';
33

44
const reactRegex = /import.*from\s['|"]react['|"]/;
5-
function hasReact(code: string) {
5+
export function hasReact(code: string) {
66
return reactRegex.test(code);
77
}
88

99
const cssRegex = /\.css$/;
1010
const jsonRegex = /\.json$/;
1111
const htmlRegex = /\.html$/;
1212

13-
function getMode(module: Module) {
13+
export function getMode(module: Module) {
1414
if (cssRegex.test(module.title)) return 'css';
1515
if (jsonRegex.test(module.title)) return 'json';
1616
if (htmlRegex.test(module.title)) return 'html';

src/sandbox/errors/no-dom-change-error.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ export default class NoDomChangeError extends Error {
1010

1111
type = 'no-dom-change';
1212
severity = 'warning';
13-
line = -1;
13+
hideLine: true;
1414
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export default class RawReactComponentError extends Error {
2+
constructor(importedModule) {
3+
super();
4+
5+
this.payload = {
6+
importedModuleId: importedModule.id,
7+
};
8+
}
9+
10+
type = 'raw-react-component-import';
11+
severity = 'error';
12+
hideLine = true;
13+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/* @flow */
2+
import type { Module } from 'common/types';
3+
import RawReactComponentError from '../../errors/raw-react-component-error';
4+
5+
import {
6+
hasReact,
7+
getMode,
8+
} from 'app/store/entities/sandboxes/modules/utils/get-type';
9+
10+
const getModule = (modules: Array<Module>, id: string): ?Module =>
11+
modules.find(m => m.id === id);
12+
13+
const reactRegex = /import.*from\s['|"]react['|"]/;
14+
/**
15+
* This error happens when a user tries to use a React component that's imported
16+
* from a raw file. In that case the React component will be a string and thus
17+
* invalid
18+
*/
19+
function buildRawReactComponentError(
20+
error: Error,
21+
module: Module,
22+
modules: Array<Module>,
23+
requires: Array<string>,
24+
) {
25+
const rawModuleUsingReact = requires
26+
.map(id => getModule(modules, id))
27+
.filter(x => x)
28+
.filter(m => hasReact(m.code || ''))
29+
.find(m => getMode(m) === 'raw');
30+
31+
if (!rawModuleUsingReact) return error;
32+
33+
return new RawReactComponentError(rawModuleUsingReact);
34+
}
35+
36+
/**
37+
* This function checks if the error is a known error, as in, it's an error
38+
* where we can show the user a better error message based on context.
39+
*
40+
* @export
41+
* @param {Error} error
42+
*/
43+
export default function transformError(
44+
error: Error,
45+
module: Module,
46+
modules: Array<Module>,
47+
requires: Array<string>,
48+
) {
49+
if (error.message.startsWith('Invalid tag: import React')) {
50+
const newError = buildRawReactComponentError(
51+
error,
52+
module,
53+
modules,
54+
requires,
55+
);
56+
newError.message = error.message;
57+
return newError;
58+
}
59+
60+
return error;
61+
}

src/sandbox/eval/js/index.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import evalModule from '../';
77
import resolveModule from '../../utils/resolve-module';
88
import resolveDependency from './dependency-resolver';
99
import getBabelConfig from './babel-parser';
10+
import transformError from './error-transformer';
1011

1112
const moduleCache = new Map();
1213

@@ -45,10 +46,16 @@ function evaluate(code, require) {
4546
const module = { exports: {} };
4647
const exports = {};
4748
const process = { env: { NODE_ENV: 'development' } }; // eslint-disable-line no-unused-vars
48-
eval(code); // eslint-disable-line no-eval
49+
try {
50+
eval(code); // eslint-disable-line no-eval
51+
52+
// Choose either the export of __esModule or node
53+
return Object.keys(exports).length > 0 ? exports : module.exports;
54+
} catch (e) {
55+
e.isEvalError = true;
4956

50-
// Choose either the export of __esModule or node
51-
return Object.keys(exports).length > 0 ? exports : module.exports;
57+
throw e;
58+
}
5259
}
5360

5461
/**
@@ -70,8 +77,8 @@ export default function evaluateJS(
7077
depth: ?number,
7178
parentModules: Array<Module>,
7279
) {
80+
const requires = [];
7381
try {
74-
const requires = [];
7582
require = function require(path: string) {
7683
// eslint-disable-line no-unused-vars
7784
if (/^(\w|@)/.test(path)) {
@@ -135,6 +142,10 @@ export default function evaluateJS(
135142
} catch (e) {
136143
// Remove cache
137144
moduleCache.delete(mainModule.id);
138-
throw e;
145+
146+
e.module = e.module || mainModule;
147+
148+
const newError = transformError(e, e.module, modules, requires);
149+
throw newError;
139150
}
140151
}

src/sandbox/utils/error-message-builder.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,32 @@ export default function buildErrorMessage(e) {
1212
const message = getMessage(e);
1313
let line = null;
1414
let column = null;
15+
if (!e.hideLine) {
16+
// Safari
17+
if (e.line != null) {
18+
line = e.line;
1519

16-
// Safari
17-
if (e.line != null) {
18-
line = e.line;
20+
// FF
21+
} else if (e.lineNumber != null) {
22+
line = e.lineNumber;
1923

20-
// FF
21-
} else if (e.lineNumber != null) {
22-
line = e.lineNumber;
23-
24-
// Chrome
25-
} else if (e.stack) {
26-
const matched = e.stack.match(/<anonymous>:(\d+):(\d+)/);
27-
if (matched) {
28-
line = matched[1];
29-
column = matched[2];
30-
} else {
31-
// Maybe it's a babel transpiler error
32-
const babelMatched = e.stack.match(/(\d+):(\d+)/);
33-
if (babelMatched) {
34-
line = babelMatched[1];
35-
column = babelMatched[2];
24+
// Chrome
25+
} else if (e.stack) {
26+
const matched = e.stack.match(/<anonymous>:(\d+):(\d+)/);
27+
if (matched) {
28+
line = matched[1];
29+
column = matched[2];
30+
} else {
31+
// Maybe it's a babel transpiler error
32+
const babelMatched = e.stack.match(/(\d+):(\d+)/);
33+
if (babelMatched) {
34+
line = babelMatched[1];
35+
column = babelMatched[2];
36+
}
3637
}
3738
}
3839
}
40+
3941
return {
4042
moduleId: e.module ? e.module.id : e.moduleId,
4143
type,

0 commit comments

Comments
 (0)