Skip to content

Commit

Permalink
fix: Permit single-line JSON comments (#1361)
Browse files Browse the repository at this point in the history
Strip comments from _locales/*/messages.json.

And set an explicit encoding in readFile so that the return value is a
string instead of a Buffer.
  • Loading branch information
Rob--W authored and rpl committed Aug 29, 2018
1 parent 9ec3a5e commit 38f6c23
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
5 changes: 3 additions & 2 deletions src/cmd/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {createWriteStream} from 'fs';

import {fs} from 'mz';
import parseJSON from 'parse-json';
import stripJsonComments from 'strip-json-comments';
import defaultEventToPromise from 'event-to-promise';

import defaultSourceWatcher from '../watcher';
Expand Down Expand Up @@ -71,14 +72,14 @@ export async function getDefaultLocalizedName(
let extensionName: string = manifestData.name;

try {
messageContents = await fs.readFile(messageFile);
messageContents = await fs.readFile(messageFile, {encoding: 'utf-8'});
} catch (error) {
throw new UsageError(
`Error reading messages.json file at ${messageFile}: ${error}`);
}

try {
messageData = parseJSON(String(messageContents), messageFile);
messageData = parseJSON(stripJsonComments(messageContents), messageFile);
} catch (error) {
throw new UsageError(
`Error parsing messages.json ${error}`);
Expand Down
5 changes: 2 additions & 3 deletions src/util/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default async function getValidatedManifest(
let manifestContents;

try {
manifestContents = await fs.readFile(manifestFile);
manifestContents = await fs.readFile(manifestFile, {encoding: 'utf-8'});
} catch (error) {
throw new InvalidManifest(
`Could not read manifest.json file at ${manifestFile}: ${error}`);
Expand All @@ -47,8 +47,7 @@ export default async function getValidatedManifest(
let manifestData;

try {
manifestData = parseJSON(stripJsonComments(manifestContents.toString()),
manifestFile);
manifestData = parseJSON(stripJsonComments(manifestContents), manifestFile);
} catch (error) {
throw new InvalidManifest(
`Error parsing manifest.json at ${manifestFile}: ${error}`);
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/test-cmd/test.build.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,33 @@ describe('build', () => {
);
});

it('handles comments in messages.json', () => {
return withTempDir(
(tmpDir) => {
const messageFileName = path.join(tmpDir.path(), 'messages.json');
fs.writeFileSync(
messageFileName,
`{"extensionName": {
"message": "example extension", // comments
"description": "example with comments"
}
}`
);

return getDefaultLocalizedName({
messageFile: messageFileName,
manifestData: {
name: '__MSG_extensionName__',
version: '0.0.1',
},
})
.then((result) => {
assert.match(result, /example extension/);
});
}
);
});

it('checks locale file for malformed json', () => {
return withTempDir(
(tmpDir) => {
Expand Down

0 comments on commit 38f6c23

Please sign in to comment.