Skip to content

Commit 9e85d14

Browse files
matthewpmatthewp
committedOct 21, 2016
Warn when error thrown parsing JSON
This does 2 things: 1) Moves system-json into this project, the old project will be deprecated. 2) When we catch JSON.parse errors (if the json has syntax errors) we return an empty object and warn the user of the error. Because we detect if a module is JSON we cannot throw in the case where JSON fails to parse. Closes #763
·
v2.3.01.3.1
1 parent 05a6875 commit 9e85d14

18 files changed

+298
-47
lines changed
 

‎Gruntfile.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ module.exports = function (grunt) {
2727
"src/system-extension-contextual.js",
2828
"src/system-extension-script-module.js",
2929
"node_modules/system-trace/trace.js",
30-
"node_modules/system-json/json.js",
30+
"src/json/json.js",
3131
"src/config.js",
3232
"node_modules/steal-env/env.js",
3333
"src/startup.js",
@@ -58,6 +58,7 @@ module.exports = function (grunt) {
5858
"src/system-extension-contextual.js",
5959
"src/system-extension-script-module.js",
6060
"node_modules/system-trace/trace.js",
61+
"src/json/json.js",
6162
"src/config.js",
6263
"node_modules/steal-env/env.js",
6364
"src/startup.js",

‎main.js

+89-2
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@
109109
},
110110
isWebWorker = typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope,
111111
isNode = typeof process === "object" && {}.toString.call(process) === "[object process]",
112-
isBrowserWithWindow = !isNode && typeof window !== "undefined";
112+
isBrowserWithWindow = !isNode && typeof window !== "undefined",
113+
warn = typeof console === "object" ? console.warn.bind(console) : function(){};
113114

114115
var filename = function(uri){
115116
var lastSlash = uri.lastIndexOf("/");
@@ -698,6 +699,93 @@ if(typeof System !== "undefined") {
698699
applyTraceExtension(System);
699700
}
700701

702+
/*
703+
SystemJS JSON Format
704+
Provides the JSON module format definition.
705+
*/
706+
function _SYSTEM_addJSON(loader) {
707+
var jsonTest = /^[\s\n\r]*[\{\[]/;
708+
var jsonExt = /\.json$/i;
709+
var jsExt = /\.js$/i;
710+
711+
// Add the extension to _extensions so that it can be cloned.
712+
loader._extensions.push(_SYSTEM_addJSON);
713+
714+
// if someone has a moduleName that is .json, make sure it loads a json file
715+
// no matter what paths might do
716+
var loaderLocate = loader.locate;
717+
loader.locate = function(load){
718+
return loaderLocate.apply(this, arguments).then(function(address){
719+
if(jsonExt.test(load.name)) {
720+
return address.replace(jsExt, "");
721+
}
722+
723+
return address;
724+
});
725+
};
726+
727+
var transform = function(loader, load, data){
728+
var fn = loader.jsonOptions && loader.jsonOptions.transform;
729+
if(!fn) return data;
730+
return fn.call(loader, load, data);
731+
};
732+
733+
// If we are in a build we should convert to CommonJS instead.
734+
if(isNode) {
735+
var loaderTranslate = loader.translate;
736+
loader.translate = function(load){
737+
if(jsonExt.test(load.name)) {
738+
var parsed = parse(load);
739+
if(parsed) {
740+
parsed = transform(this, load, parsed);
741+
return "def" + "ine([], function(){\n" +
742+
"\treturn " + JSON.stringify(parsed) + "\n});";
743+
}
744+
}
745+
746+
return loaderTranslate.call(this, load);
747+
};
748+
return;
749+
}
750+
751+
var loaderInstantiate = loader.instantiate;
752+
loader.instantiate = function(load) {
753+
var loader = this,
754+
parsed;
755+
756+
parsed = parse(load);
757+
if(parsed) {
758+
parsed = transform(loader, load, parsed);
759+
load.metadata.format = 'json';
760+
761+
load.metadata.execute = function(){
762+
return parsed;
763+
};
764+
}
765+
766+
return loaderInstantiate.call(loader, load);
767+
};
768+
769+
return loader;
770+
771+
// Attempt to parse a load as json.
772+
function parse(load){
773+
if ( (load.metadata.format === 'json' || !load.metadata.format) && jsonTest.test(load.source) ) {
774+
try {
775+
return JSON.parse(load.source);
776+
} catch(e) {
777+
warn("Error parsing " + load.address + ":", e);
778+
return {};
779+
}
780+
}
781+
782+
}
783+
}
784+
785+
if (typeof System !== "undefined") {
786+
_SYSTEM_addJSON(System);
787+
}
788+
701789
// Overwrites System.config with setter hooks
702790
var setterConfig = function(loader, configOrder, configSpecial){
703791
var oldConfig = loader.config;
@@ -1448,7 +1536,6 @@ if (typeof System !== "undefined") {
14481536
steal.clone = cloneSteal;
14491537
module.exports = global.steal;
14501538
global.steal.addSteal = addSteal;
1451-
require("system-json");
14521539

14531540
} else {
14541541
var oldSteal = global.steal;

‎package.json

+21-21
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
"dependencies": {
1515
"less": "^2.4.0",
1616
"steal-systemjs": "0.18.4",
17-
"system-json": "^0.1.0",
1817
"console-browserify": "~1.1.0",
1918
"constants-browserify": "~0.0.1",
2019
"crypto-browserify": "~2.1.10",
@@ -50,9 +49,10 @@
5049
"steal-env": "^1.0.0",
5150
"steal-es6-module-loader": "0.17.9",
5251
"steal-less": "0.4.0-pre.1",
52+
"steal-npm": "1.0.0-alpha.0",
53+
"steal-qunit": "^0.1.4",
5354
"system-bower": "stealjs/system-bower#v0.2.1",
5455
"system-live-reload": "1.5.3",
55-
"steal-npm": "1.0.0-alpha.0",
5656
"system-trace": "0.2.5",
5757
"testee": "^0.2.0",
5858
"traceur": "0.0.91",
@@ -98,25 +98,25 @@
9898
"_stream_passthrough": "./ext/builtin/_stream_passthrough.js"
9999
},
100100
"system": {
101-
"npmDependencies": [
102-
"console-browserify",
103-
"constants-browserify",
104-
"crypto-browserify",
105-
"http-browserify",
106-
"buffer",
107-
"os-browserify",
108-
"vm-browserify",
109-
"zlib-browserify",
110-
"assert",
111-
"domain-browser",
112-
"events",
113-
"https-browserify",
114-
"path-browserify",
115-
"string_decoder",
116-
"tty-browserify",
117-
"process",
118-
"punycode"
119-
]
101+
"npmDependencies": [
102+
"console-browserify",
103+
"constants-browserify",
104+
"crypto-browserify",
105+
"http-browserify",
106+
"buffer",
107+
"os-browserify",
108+
"vm-browserify",
109+
"zlib-browserify",
110+
"assert",
111+
"domain-browser",
112+
"events",
113+
"https-browserify",
114+
"path-browserify",
115+
"string_decoder",
116+
"tty-browserify",
117+
"process",
118+
"punycode"
119+
]
120120
},
121121
"repository": {
122122
"type": "git",

‎src/end.js

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
steal.clone = cloneSteal;
88
module.exports = global.steal;
99
global.steal.addSteal = addSteal;
10-
require("system-json");
1110

1211
} else {
1312
var oldSteal = global.steal;

‎src/json/json.js

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
SystemJS JSON Format
3+
Provides the JSON module format definition.
4+
*/
5+
function _SYSTEM_addJSON(loader) {
6+
var jsonTest = /^[\s\n\r]*[\{\[]/;
7+
var jsonExt = /\.json$/i;
8+
var jsExt = /\.js$/i;
9+
10+
// Add the extension to _extensions so that it can be cloned.
11+
loader._extensions.push(_SYSTEM_addJSON);
12+
13+
// if someone has a moduleName that is .json, make sure it loads a json file
14+
// no matter what paths might do
15+
var loaderLocate = loader.locate;
16+
loader.locate = function(load){
17+
return loaderLocate.apply(this, arguments).then(function(address){
18+
if(jsonExt.test(load.name)) {
19+
return address.replace(jsExt, "");
20+
}
21+
22+
return address;
23+
});
24+
};
25+
26+
var transform = function(loader, load, data){
27+
var fn = loader.jsonOptions && loader.jsonOptions.transform;
28+
if(!fn) return data;
29+
return fn.call(loader, load, data);
30+
};
31+
32+
// If we are in a build we should convert to CommonJS instead.
33+
if(isNode) {
34+
var loaderTranslate = loader.translate;
35+
loader.translate = function(load){
36+
if(jsonExt.test(load.name)) {
37+
var parsed = parse(load);
38+
if(parsed) {
39+
parsed = transform(this, load, parsed);
40+
return "def" + "ine([], function(){\n" +
41+
"\treturn " + JSON.stringify(parsed) + "\n});";
42+
}
43+
}
44+
45+
return loaderTranslate.call(this, load);
46+
};
47+
return;
48+
}
49+
50+
var loaderInstantiate = loader.instantiate;
51+
loader.instantiate = function(load) {
52+
var loader = this,
53+
parsed;
54+
55+
parsed = parse(load);
56+
if(parsed) {
57+
parsed = transform(loader, load, parsed);
58+
load.metadata.format = 'json';
59+
60+
load.metadata.execute = function(){
61+
return parsed;
62+
};
63+
}
64+
65+
return loaderInstantiate.call(loader, load);
66+
};
67+
68+
return loader;
69+
70+
// Attempt to parse a load as json.
71+
function parse(load){
72+
if ( (load.metadata.format === 'json' || !load.metadata.format) && jsonTest.test(load.source) ) {
73+
try {
74+
return JSON.parse(load.source);
75+
} catch(e) {
76+
warn("Error parsing " + load.address + ":", e);
77+
return {};
78+
}
79+
}
80+
81+
}
82+
}
83+
84+
if (typeof System !== "undefined") {
85+
_SYSTEM_addJSON(System);
86+
}

‎src/json/json_test.js

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
QUnit.module("JSON support");
2+
3+
asyncTest("Basics works", function(){
4+
System.import("src/json/tests/my.json").then(function(my){
5+
equal(my.name, "foo", "name is right");
6+
}).then(start);
7+
});
8+
9+
asyncTest("Still resolves when we fail to parse", function(){
10+
System.import("src/json/tests/bad.json").then(function(){
11+
ok(true);
12+
}).then(start);
13+
});
14+
15+
asyncTest("jsonOptions transform allows you to transform the json object", function(){
16+
System.jsonOptions = {
17+
transform: function(load, data){
18+
delete data.priv;
19+
return data;
20+
}
21+
};
22+
23+
System.import("src/json/tests/another.json").then(function(a){
24+
ok(!a.priv, "Private field excluded");
25+
}).then(start);
26+
});

‎src/json/test.html

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!doctype html>
2+
<title>Steal json tests</title>
3+
<link rel="stylesheet" type="text/css" href="../../node_modules/qunitjs/qunit/qunit.css"/>
4+
5+
<h1 id="qunit-header">Steal Unit Test Suite</h1>
6+
7+
<h2 id="qunit-banner"></h2>
8+
<div id="qunit-testrunner-toolbar"></div>
9+
<h2 id="qunit-userAgent"></h2>
10+
<ol id="qunit-tests"></ol>
11+
<div id="qunit-test-area"></div>
12+
13+
14+
<script src="../../node_modules/qunitjs/qunit/qunit.js"></script>
15+
<script src="../../steal.js" base-url="../../" config-main="@empty" main="@empty"></script>
16+
<script>
17+
QUnit.config.autostart = false;
18+
19+
steal.import("src/json/json_test").then(function(){
20+
QUnit.start();
21+
});
22+
</script>

‎src/json/tests/alt_path/bower.json

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "foo",
3+
"main": "foo.js",
4+
"system": {
5+
"paths": {
6+
"bar": "bower_components/bar/bar.js"
7+
}
8+
}
9+
}

‎src/json/tests/another.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "foo",
3+
"priv": "PRIVATE"
4+
}

‎src/json/tests/bad.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
// Comments aren't allowed, duh!
3+
"foo": "bar"
4+
}

‎src/json/tests/build_config/bower.json

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"name": "main",
3+
"main": "foo.js",
4+
"system": {
5+
"buildConfig": {
6+
"map": {
7+
"foo": "bar"
8+
}
9+
}
10+
}
11+
}

‎src/json/tests/my.json

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"name" : "foo"}

‎src/start.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,5 @@
109109
},
110110
isWebWorker = typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope,
111111
isNode = typeof process === "object" && {}.toString.call(process) === "[object process]",
112-
isBrowserWithWindow = !isNode && typeof window !== "undefined";
112+
isBrowserWithWindow = !isNode && typeof window !== "undefined",
113+
warn = typeof console === "object" ? console.warn.bind(console) : function(){};

‎steal.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -5161,7 +5161,8 @@ var $__curScript, __eval;
51615161
},
51625162
isWebWorker = typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope,
51635163
isNode = typeof process === "object" && {}.toString.call(process) === "[object process]",
5164-
isBrowserWithWindow = !isNode && typeof window !== "undefined";
5164+
isBrowserWithWindow = !isNode && typeof window !== "undefined",
5165+
warn = typeof console === "object" ? console.warn.bind(console) : function(){};
51655166

51665167
var filename = function(uri){
51675168
var lastSlash = uri.lastIndexOf("/");
@@ -5758,7 +5759,6 @@ function _SYSTEM_addJSON(loader) {
57585759
var jsonTest = /^[\s\n\r]*[\{\[]/;
57595760
var jsonExt = /\.json$/i;
57605761
var jsExt = /\.js$/i;
5761-
var inNode = typeof window === "undefined";
57625762

57635763
// Add the extension to _extensions so that it can be cloned.
57645764
loader._extensions.push(_SYSTEM_addJSON);
@@ -5783,7 +5783,7 @@ function _SYSTEM_addJSON(loader) {
57835783
};
57845784

57855785
// If we are in a build we should convert to CommonJS instead.
5786-
if(inNode) {
5786+
if(isNode) {
57875787
var loaderTranslate = loader.translate;
57885788
loader.translate = function(load){
57895789
if(jsonExt.test(load.name)) {
@@ -5825,7 +5825,10 @@ function _SYSTEM_addJSON(loader) {
58255825
if ( (load.metadata.format === 'json' || !load.metadata.format) && jsonTest.test(load.source) ) {
58265826
try {
58275827
return JSON.parse(load.source);
5828-
} catch(e) {}
5828+
} catch(e) {
5829+
warn("Error parsing " + load.address + ":", e);
5830+
return {};
5831+
}
58295832
}
58305833

58315834
}
@@ -6585,7 +6588,6 @@ if (typeof System !== "undefined") {
65856588
steal.clone = cloneSteal;
65866589
module.exports = global.steal;
65876590
global.steal.addSteal = addSteal;
6588-
require("system-json");
65896591

65906592
} else {
65916593
var oldSteal = global.steal;

‎steal.production.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/test.html

+2-6
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,10 @@ <h2 id="qunit-userAgent"></h2>
1414
<ol id="qunit-tests"></ol>
1515
<div id="qunit-test-area"></div>
1616

17-
<script src="../node_modules/steal-es6-module-loader/dist/es6-module-loader.src.js" type="text/javascript"></script>
18-
<script src="../node_modules/steal-systemjs/dist/system.src.js" data-traceur-src="../node_modules/traceur/bin/traceur.js" type="text/javascript"></script>
19-
<script src="../system-format-steal.js"></script>
17+
<script src="../steal.js" config-main="@empty" main="@empty"></script>
2018
<script src="../node_modules/qunitjs/qunit/qunit.js"></script>
2119

2220
<script src="test.js"></script>
23-
<script>
24-
//delete window.define;
25-
</script>
21+
<script src="../src/json/json_test.js"></script>
2622
</body>
2723
</html>

‎test/test.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ QUnit.config.testTimeout = 30000;
1616
}
1717
})();
1818

19+
System.baseURL = "../";
20+
1921
var writeIframe = function(html){
2022
var iframe = document.createElement('iframe');
2123
window.removeMyself = function(){
@@ -63,7 +65,7 @@ QUnit.config.testTimeout = 30000;
6365
};
6466

6567
asyncTest('steal basics', function(){
66-
System['import']('tests/module').then(function(m){
68+
System['import']('test/tests/module').then(function(m){
6769
equal(m.name,"module.js", "module returned" );
6870
equal(m.bar.name, "bar", "module.js was not able to get bar");
6971
start();
@@ -74,7 +76,7 @@ QUnit.config.testTimeout = 30000;
7476
});
7577

7678
asyncTest("steal's normalize", function(){
77-
System['import']('tests/mod/mod').then(function(m){
79+
System['import']('test/tests/mod/mod').then(function(m){
7880
equal(m.name,"mod", "mod returned" );
7981
equal(m.module.bar.name, "bar", "module.js was able to get bar");
8082
equal(m.widget(), "widget", "got a function");
@@ -110,8 +112,8 @@ QUnit.config.testTimeout = 30000;
110112
});
111113

112114
asyncTest("ignoring an import by mapping to @empty", function(){
113-
System.map["map-empty/other"] = "@empty";
114-
System["import"]("map-empty/main").then(function(m) {
115+
System.map["test/map-empty/other"] = "@empty";
116+
System["import"]("test/map-empty/main").then(function(m) {
115117
var empty = System.get("@empty");
116118
equal(m.other, empty, "Other is an empty module because it was mapped to empty in the config");
117119
}, function(){
@@ -120,17 +122,17 @@ QUnit.config.testTimeout = 30000;
120122
});
121123

122124
asyncTest("steal.dev.assert", function() {
123-
System["import"]("ext/dev").then(function(dev){
125+
System["import"]("@dev").then(function(){
124126
throws(
125127
function() {
126-
dev.assert(false);
128+
steal.dev.assert(false);
127129
},
128130
/Expected/,
129131
"throws an error with default message"
130132
);
131133
throws(
132134
function() {
133-
dev.assert(false, "custom message");
135+
steal.dev.assert(false, "custom message");
134136
},
135137
/custom message/,
136138
"throws an error with custom message"

‎test/tests/mod/mod.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
steal("../module", "tests/mod/widget",function(m, w){
1+
steal("../module", "test/tests/mod/widget",function(m, w){
22
return {
33
module: m,
44
widget: w,

0 commit comments

Comments
 (0)
Please sign in to comment.