Skip to content
This repository was archived by the owner on Dec 22, 2020. It is now read-only.

Avoid swallowing JSON5 error output #34

Merged
merged 2 commits into from
Mar 22, 2019
Merged

Avoid swallowing JSON5 error output #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f049f69
Remove try/catch throw to avoid swallowing JSON5 error output
JakobJingleheimer Mar 19, 2019
293cbed
Switch to emitError
JakobJingleheimer Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,7 @@ import util from 'util';
import JSON5 from 'json5';

function Json5Loader(source) {
let value;

try {
value = JSON5.parse(source);
} catch (e) {
throw new Error('Error parsing JSON5', (e));
}
Copy link
Contributor Author

@JakobJingleheimer JakobJingleheimer Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not doing anything except swallowing JSON5's error info. An error is thrown either way, so might as well do nothing and get the useful info.

Copy link
Member

@alexander-akait alexander-akait Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need change throw Error on https://webpack.js.org/api/loaders/#thisemiterror

Copy link
Contributor Author

@JakobJingleheimer JakobJingleheimer Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean switch it from throwing entirely (even though it was not emitting before)?

Copy link
Member

@alexander-akait alexander-akait Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before loader swallow error, with this.emitError it will work as expected. Just:

try {	
    value = JSON5.parse(source);	
  } catch (e) {	
    this.emitError(e);	
  }

Copy link
Contributor Author

@JakobJingleheimer JakobJingleheimer Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Will do when I get home from work.

const value = JSON5.parse(source);

return `module.exports = ${util.inspect(value, { depth: null })}`;
}
Expand Down
2 changes: 1 addition & 1 deletion test/json5-loader.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe(PROJECT_NAME, () => {
const brokenJson5 = '{broken: json5}';
expect(() => {
Json5Loader.call({}, brokenJson5);
}).toThrow('Error parsing JSON5');
}).toThrow(SyntaxError);
done();
});

Expand Down