Loading grammar with highlights throws missing scopeName property error

[austinkelleher]

Description

We are using the language-javascript package (along with several others) in combination with highlights to syntax highlight a REPL on a webpage. It seems that sometime recently new grammar files were added without a scopeName property (e.g. https://github.com/atom/language-javascript/blob/master/grammars/tree-sitter-javascript.cson) and instead are using legacyScopeName. This is causing an error to be thrown from first-mate as it is expecting a scopeName property. I'm not sure what the right solution is to this. We could either:

1. Add a scopeName property to each of the grammar files that is missing it
2. Add support for legacyScopeName in this module

I'm leaning towards 2 as the better solution, but I'm not really sure why legacyScopeName was introduced in the first place. I would be happy to work on this and submit a PR given proper feedback on the above information.

Case:

const Highlights = require("highlights");

const highlighter = new Highlights();

highlighter.requireGrammarsSync({
  modulePath: require.resolve("language-javascript/package.json")
});

Throws the following error:

Caused by: Error: Grammar missing required scopeName property: /node_modules/language-javascript/grammars/tree-sitter-javascript.cson
  at GrammarRegistry.module.exports.GrammarRegistry.readGrammarSync (/node_modules/first-mate/lib/grammar-registry.js:107:15)
  at GrammarRegistry.module.exports.GrammarRegistry.loadGrammarSync (/node_modules/first-mate/lib/grammar-registry.js:133:22)
  at Highlights.module.exports.Highlights.requireGrammarsSync (/node_modules/highlights/lib/highlights.js:95:25)

/cc @50Wliu @maxbrunsfeld

[winstliu]

Tracking in atom/atom#16581.

[winstliu]

Actually that is related but not quite the same.

[austinkelleher]

Do have any thoughts on the two potential fixes I suggested @50Wliu?

[winstliu]

I will defer to @maxbrunsfeld who is working on tree-sitter.

[austinkelleher]

@maxbrunsfeld Any thoughts on this? This bug is currently breaking our REPL for: https://markojs.com/try-online/

[austinkelleher]

Ping @maxbrunsfeld @50Wliu following up with this issue since there is still no consensus. Thoughts on my suggested fixes? #106 (comment)