limbo-works / Limbo.Umbraco.ModelsBuilder

Custom models builder for Umbraco.
MIT License
4 stars 3 forks source link

Investigate deleting files before generating models #10

Closed abjerner closed 2 years ago

abjerner commented 2 years ago

See: https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/7#issuecomment-1083591996

hfloyd commented 2 years ago

Copied from @abjerner's comment https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/7#issuecomment-1083591996:

We've had some talks about this internally as well, but no conclusion yet.

With OMB, the models were always placed in a single folder. With LMB, models can split out and placed in different folders, which makes it a bit harder to keep track of.

Given the file header added by LMB, I'd imagine I could find all *.generated.cs file in the project generated by LMB, and then delete them before generating the models again.

I've also considered adding the content type GUID to the file somewhere. If so, this could allow the generator to only delete the files it needs to. But it may not work if content types are deleted.

ASP.NET Core also works a bit different from ASP.NET, as updating files could trigger a restart. I haven't experienced problems with this so far, but maybe there could be problems when deleting all generated files first, and then generating the files all over again 🤷‍♂️

It's definitely something I will look a bit more into when I have some more time to work on the package 😉

Copied from @abjerner's comment https://github.com/limbo-works/Limbo.Umbraco.ModelsBuilder/issues/7#issuecomment-1084832067:

Turns out what I described actually causes more problems that I had thought.

When developing on this package so far, I've used a mix running my test site directly from VS2019 and running it from console via dotnet run. When working this way, I didn't experience any problems. When running from VS2019, the site will restart the LMB saves the updated models to disk, but all files seem to be saved before the server restarts.

On the other hand, when running the site from either VS2022 or via dotnet watch run - possibly because of hot reloading - the site restarts before all model files are saved to disk, so the process doesn't complete.

I've yet to find a solution for this. It seems that it works fine with EMB, but haven't spotted any major differences when running through their code - but I need to look at bit more in to whether they're doing something I've missed.

hfloyd commented 2 years ago

I have been running my local site (where this package is installed) via IIS (as per https://our.umbraco.com/documentation/Fundamentals/Setup/Install/iis) but just letting it run directly from IIS and a web browser (not using the "Debug" option in VS2022), so I haven't noticed any issues...

Maybe in terms of dealing with "obsolete" files, there is another way to alert devs that file is out-of-date, without having to delete it directly. Perhaps via the additional generation of some sort of "status" file? Ex:

{
  "ModelsGenerated": [
    "MyDoctypeA",
    "MyDocTypeB",
    "..."
  ],
  "ObsoleteFilesToDelete": [
    "OldDoctypeA.generated.cs",
    "OldDocTypeB.generated.cs",
    "..."
  ]
}

Just a thought...

abjerner commented 2 years ago

Hi @hfloyd

I've now updated the source generator to look through the models directory and delete all *.generated.cs files with a ModelsBuilder file header.

I suspect keeping a JSON file to track the generated files could lead to a few problems - eg. when working multiple developers on the same repo.

The delete logic can be enabled or disabled via the DeleteGeneratedFiles setting in appsettings.json. It's enabled by default now.


If you do run your site from VS or via dotnet watch, you can tell the watcher to ignore generated files - eg.:

<ItemGroup>
  <Compile Remove="Models\Umbraco\**\*.generated.cs"/>
  <Compile Include="Models\Umbraco\**\*.generated.cs" Watch="false" />
</ItemGroup>

This seems to address the issues I described earlier 😍


I haven't pushed a new release yet, but I'd imagine I'm doing that a bit alter today.

hfloyd commented 2 years ago

@abjerner Cool! I'll have to test it once I remove some Doctypes ;-)