Replace relaxed-json with strip-json-comments

[fregante]

The MDN documentation mentions that:

[manifest.json] is a JSON-formatted file, with one exception: it is allowed to contain "//"-style comments.

But RJSON is a bit more than that: https://github.com/phadej/relaxed-json#relaxed-json

Given this and that relaxed-json has a few CLI-related dependencies, I'd suggest swapping it out for the more appropriate strip-json-comments, which has no dependencies.

Related:

• Reduce dependencies #3601

┆Issue is synchronized with this Jira Task

[fregante]

In general, I'm a bit confused about this file:

addons-linter/src/parsers/json.js

Line 30 in 872132e

// First we'll try to remove comments with esprima;

Why is esprima being used before RJSON at all? It seems that the whole .parse method should just be a plain call to parse: json => JSON.parse(stripJsonComments(json))

Or perhaps it should use strip-json-comments in conjunction with https://www.npmjs.com/package/json-parse-even-better-errors for even better errors

[fregante]

Side note: both espree and esprima are used in this project, so esprima should probably be dropped anyway (because eslint still loads espree )

[Rob--W]

RelaxedJSON is not used for parsing, but only as an attempt to get useful error messages. We can use JSON.parse, and if that fails, strip the comments and retry, and if that fails, try to extract a useful message from the original error (with line numbers for example).

For manifest.json and _locales/<locale>/messages.json, replacing the overly broad RJSON with stripJsonComments+JSON.parse should be safe, because Firefox itself already strips comments (without allowing a broader format): https://searchfox.org/mozilla-central/rev/4d90ff4537330d6b17cb956c0fadf759086d9bb7/toolkit/components/extensions/Extension.jsm#268-281

The linter also lints arbitrary JSON files:

• JSONParser uses RelaxedJSON:

  addons-linter/src/parsers/json.js

  Line 6 in 872132e

  export default class JSONParser {

  
  

  addons-linter/src/parsers/json.js

  Line 26 in 872132e

  parse(RelaxedJSON = RJSON) {

• used here:

  addons-linter/src/scanners/json.js

  Line 30 in 872132e

  const jsonParser = new JSONParser(

• referenced here:

  addons-linter/src/linter.js

  Lines 353 to 354 in 47c3a11

  	case '.json':
  	return JSONScanner;

Perhaps we can disable JSON parsing for non-standard JSON files (i.e. not used by the extension framework in a special way). That would also resolve #3680

[stale]

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

[fregante]

So I suggest replacing esprima and RJSON in favor of a combination of:

• strip-json-comments
• json-parse-even-better-errors

They're both light, dependency-free and more focused, whereas the other two:

• https://packagephobia.com/result?p=rjson
• https://packagephobia.com/result?p=esprima

[fregante]

So it turns out this is really simple:

export const RJSON = {
  parse(jsonString) {
    return JSON.parse(stripJsonComments(jsonString));
  },
};

That's all that's needed… except that RJSON can detect duplicate keys and addons-linter specifically tests for them.

I don't know of a way to detect duplicate keys other than by parsing the JSON file manually (e.g via something like relaxed-json). I also tried JSON.parse's reviver function but that also skips the duplicates.

Either the duplicate detection is dropped, or we cannot drop relaxed-json :(

The unfortunate part is that relaxed-json hasn't seen updates in 5 years and it has a few heavy old deps