Lombiq/Helpful-Libraries

Add source generator that reads a JSON value as a const string at compile time. (OSOE-819)

Closed this issue · 10 comments

Disclaimer: I'm not sure if this is the right repo for this suggestion. I don't think we have done any source generators yet, so maybe it should be a new repo if we do this. The idea ties into node/npm but it's not dependent on Node Extensions.

Problem

When you include a vendor resource in OC, it's a good idea to include the version too, for the purpose of cache busting. For example

        _manifest
            .DefineScript(Library)
            .SetUrl(Vendors + "chart.js/Chart.min.js", Vendors + "chart.js/Chart.js")
            .SetVersion("2.9.4");

As you can see, the version here is just a raw string. It's derived from the matching "chart.js": "2.9.4", in package.json. Then later you update the package file and forget to update the matching version number in the resource manifest! It's a maintenance nightmare.

Solution

Create a source generator that creates a compile time constant. For instance:

[ConstantFromJson("ChartJsVersion", "package.json", "$..['chart.js']")]
public class ResourceManagementOptionsConfiguration : IConfigureOptions<ResourceManagementOptions>
{
    // ...
        _manifest
            .DefineScript(Library)
            .SetUrl(Vendors + "chart.js/Chart.min.js", Vendors + "chart.js/Chart.js")
            .SetVersion(ChartJsVersion);
    // ...
}

Here [ConstantFromJson(variableName, fileName, jsonPath)] would instruct the source generator to insert string constant called variableName and set its value to the first result in fileName that matches jsonPath. Then at compile time it will become private const ChartJsVersion = "2.9.4";. Of course, there should be a shortcut like [ValueFromNpm("chart.js")] as well.

I looked for existing projects, but this was the only thing vaguely similar.

Alternatives

You could mark the package.json an embedded file, then read & parse it rune time at every startup.
Just writing that sentence down made me feel ill.

Jira issue

Hmm, very interesting! This would be especially useful for updates done by Dependabot.

I've been looking into this issue and playing around with source generators as I've never worked on one before.
There are a few findings that I wanted to share here and get some feedback

The classes that will be marked with the attribute [ConstantFromJson("ChartJsVersion", "package.json", "$..['chart.js']")] will need to be marked as partial: also see Source Generators cookbook
I am assuming that it won't be an issue to mark them as partial but wanted to just call it out.

Also in order to make the package.json file accessible to the generator it would have to be marked as an "AdditionalFiles" in the .csproj as source generators do not have access to the filesystem directly.

I am setting this up as a separate project within this repo for now, let me know if there are any thoughts around that.

Partial shouldn't be an issue, nor AdditionalFiles I think but the latter also affects NuGet publishing. Perhaps it'd only result in an unnecessary file being bundled in the package, which is not a significant issue, but I don't know if it can break anything else.

Hey @Piedone,

The marking of the AdditionalFiles is actually for the projects that will use the generator not on the generator itself. So the consuming projects would have to mark those files in their own .csproj so that the generator can access them at compile time

Yeah, I understood it as such. Just that we publish all of our open-source projects using package.jsons on NuGet, so that'll affect their NuGet publishing (e.g. https://github.com/Lombiq/Orchard-Chart.js, https://github.com/Lombiq/Orchard-Content-Editors, https://github.com/Lombiq/Orchard-Data-Tables).

I get build errors locally with a fresh clone of OSOCE:

Build log.txt

image

Repeated builds also fail.

The earlier OSOCE 509442daa9bdc7dc060d8a0196b8e803881af293 also shows this:

image

Considering that it works with dotnet but not with VS, perhaps this is a VS bug. Then @AydinE please make a bug report.

Good job, @AydinE!

Reopening, since this breaks NuGet publishing.