Support json5 for Shared Configs

[RichiCoder1]

What would you like Renovate to be able to do?

We'd like to be able to add comments to make it clear why certain config settings are the way they are. This is especially important with a shared, collaborative configuration. Currently this is not possible with json. Either yaml or json5 would solve this.

Did you already have any implementation ideas?

From reading the code, two things would need to happen: A consumer would need to explictly specify the file extension (already possible today?) and the config fetcher would need to know how to parse it by toggling off the extension. An even better exp would probably try the various paths and only fail if no matching extension is found. Likely would be expensive and noisy though.

[rarkins]

I'd prefer json5, otherwise we introduce the problem of needing to document and give yaml examples for our config fields.

We also don't want to try/fail/try with file names anymore than we do today because it will increase API calls.

We therefore may want to reconsider our approach to default.json/renovate.json at the same time as this. Eg deprecating our support for "falling back" to renovate.json presets when default.json isn't found, and instead we insist that if the filename is unspecified then it must be default.json. Then we introduce syntax for specifying the full file name and extension.

[RichiCoder1]

So basically for github>foo/bar look for default.json and fall back to renovate.json with a deprecation warning/eventual removal.

For github>foo/bar:xyz, it will look for xyz.json. Edit: or were you saying deprecate this too and warn?

For github>foo/bar:xyz.json5, look for xyz.json5?

[rarkins]

All the above are correct :xyz already means xyz.json so no change there

[HonkingGoose]

Maybe the title of the issue should be changed to: "Support json5 for shared configs", as I think the Renovate team has said they prefer json5?

[felixfbecker]

YAML would be much more useful than JSON5 for multi-line template strings, which are a pain to write in JSON.

If API calls are a concern, maybe Renovate could deprecate (and auto-migrate) config with the .json5 extension and just always parse .json files using JSON5 (which should be compatible)?
Then you "free up" an API call for .yaml.

[rarkins]

As soon as we support yaml we'll have someone else complaining that we don't show yaml alternatives for every documentation example. If you can solve that problem then I'd be more open to it. I think API calls are avoidable and it a concern if we insist presets must be specified with file extension if not ending in JSON.

[bmuenzenmeyer]

Per my comments in #10100 I am interested in this work.
Per the contribution guidelines, I am signaling interest and have some initial help from @viceice

I'd like to take a short moment to show you some code and clarify decision decisions - intent is to understand if I am on the right track before I scale it up to all presets and add tests.

Taking cues from @viceice - I found getPreset (also repeated within each platform.... gitlab, azure, etc) and more importantly the eventual helper - fetchJSONFile

export function getPresetFromEndpoint(
  pkgName: string,
  filePreset: string,
  presetPath: string,
  endpoint = Endpoint
): Promise<Preset> {
  return fetchPreset({
    pkgName,
    filePreset,
    presetPath,
    endpoint,
    fetch: fetchJSONFile,
  });
}

Not wanting to ruin the semantics of fetchJSONFile, I replaced with this:

export function getPresetFromEndpoint(
  pkgName: string,
  filePreset: string,
  presetPath: string,
  endpoint = Endpoint
): Promise<Preset> {
  return fetchPreset({
    pkgName,
    filePreset,
    presetPath,
    endpoint,
-   fetch: fetchJSONFile,
+   fetch: fetchConfigFile,
  });
}

this new function abstracts the fetching of config, preserving the signature:

export async function fetchConfigFile(repo: string, fileName: string, endpoint: string): Promise<Preset> {
  if(fileName.endsWith('.json')) {
    return fetchJSONFile(repo, fileName, endpoint)
  }
  if(fileName.endsWith('.json5')) {
    return fetchJSON5File(repo, fileName, endpoint)
  }
  throw new Error(PRESET_NOT_FOUND)
}

🤔 Design decision in need of validation 🤔

The separation into a separate function should be upfront confirmed. It wouldn't be too hard to make fetchJSONFile also support JSON5 - but at that point it needs to be renamed IMO and I was less comfortable with that.

Some smaller self-nits:

• Assuming we carry on with the above, can I use node internals like path within renovate?
• Assuming we carry on with the above, should this be a switch to accommodate other potential options (like .yaml) (I'd think no... but want to avoid if if if if someday and will defer to y'all)

🤔 Question in need of validation 🤔

I also noticed this common implementation which is in use in a number of platforms. So I suspect any logic needs to be applied here, and then custom in {platform}/index.ts as needed (and now I am realizing that my github/index.ts foray was one of the exceptions to common.ts

---

thanks for your time and consideration here!

[rarkins]

Awesome work! I think one deciding factor for your questions on how best to redesign is: which would work best if we were in the future to support JSON, JSON5, and YAML?

Totally fine if you find that the system could do with a refactoring first, i.e. it's not necessary to keep changes to a minimum - although your work to do that is much appreciated. In such a case I find this approach works best:

• Make all your changes, including refactoring, to produce the best end solution. Be confident that it works.
• Now, abstract the refactoring part into a first PR, with the idea being that it's 100% backwards compatible with the existing one
• Once the refactoring is merged, your feature PR should be quite simple

Re: the common implementation, @zharinov was doing some work to improve coded reuse across platforms instead of re-implementing "fetch a JSON preset file" logic for every platform. It's possible that work wasn't finished..

Thanks for your detailed suggestion here and please don't hesitate to follow up

[viceice]

I think for JSON5 we can simply parse normal renovate.json file with json5 parser. This will be fully backward compatible ane needs less changes.

I don't see any security concerns using JSON5 by default, as it won't allow any executable code like normal JSON.

[svenXY]

Hi, having happily migrated all my renovate.json to renovate.json5, I was more than surprised to then find out (the hard way) that my presets must be json.

Is there any reason except time why this has not been further pursued? Thanks

[rarkins]

No reason except time. I think we could refactor to use the JSON5 parser for presets too. PRs welcome

[renovate-release]

🎉 This issue has been resolved in version 32.35.0 🎉

The release is available on:

• GitHub release
• 32.35.0

Your semantic-release bot 📦🚀

[karlhorky]

This is great! Thanks @viceice and @rarkins for getting this in!

Would it be possible to have a default.json5 file in the config presets filenames now? (just for editor tooling like syntax highlighting, formatting, etc)

If I should open a new issue for this, happy to do so.

[rarkins]

The answer is no if it means increasing the number of API try/catch/try API requests we do. AFAIK we today already have to try default.json and if it's missing then renovate.json, and that's something we want to deprecate already. So adding an additional check for default.json5 would not be great.

Instead, if we can extend our syntax so that you can explicitly specify the filename, then that would be fine. That way there's only one API call. Best to discuss in a new issue, if none already exist (I know we've discussed it before but not sure if we have a feature request open)

[karlhorky]

Ok, I looked a bit and didn't find any issues, so opening this new one: #15370