Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AddOrchardCms overload with a configure action. #2005

Merged
merged 6 commits into from Jun 19, 2018
Merged

Conversation

jtkech
Copy link
Member

@jtkech jtkech commented Jun 18, 2018

Here i also suggest to re-introduce this line in OC.Cms.Web.csproj so that it fixes #1156.

<MvcRazorExcludeRefAssembliesFromPublish>false</MvcRazorExcludeRefAssembliesFromPublish>

Normally we don't need it but if a module didn't use the Sdk.Razor and / or forgot a direct reference to Microsoft.AspNetCore.Mvc, its views are not precompiled and then this line is needed. This because its views will be compiled at runtime, so the refs folder with compilation libraries needs to be generated when publishing the application. So, let me know.

@sebastienros
Copy link
Member

I reverted the change that was not necessary.

@@ -7,6 +7,7 @@
<TieredCompilation>true</TieredCompilation>
<PreserveCompilationContext>true</PreserveCompilationContext>
<MvcRazorCompileOnPublish>true</MvcRazorCompileOnPublish>
<MvcRazorExcludeRefAssembliesFromPublish>false</MvcRazorExcludeRefAssembliesFromPublish>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

@jtkech
Copy link
Member Author

jtkech commented Jun 19, 2018

@sebastienros

  • So, i confirm that we still need to publish razor pages (not views) defined by the application even if they are precompiled. It is required to be found and selected by mvc even if the file only contains a @page (which is required) directive which proves that at the end this is the precompiled page which is used.

  • Then i was also publishing the razor views imported as Linked Items in the application which were not precompiled by the old tool. But i assume that it is done by the new Sdk.Razor, so i removed this part. Anyway for this case we would need to publish again the refs folder that we decided not to do.

  • Update There is also PreserveCompilationContext = true which is also related to razor compilation at runtime, but we need it while in dev mode. But just saw that now Sdk.Web set it to true by default. So i also removed this line from our web applications project files.

  • The last case i didn't tried is a razor page (not a view) imported as a Linked Item in the application. But it's too much ;)

Otherwise this PR is ready.

@sebastienros
Copy link
Member

FYI dotnet/AspNetCore.Docs#6746

@sebastienros sebastienros merged commit 51bbcf9 into dev Jun 19, 2018
@sebastienros sebastienros deleted the addOrchardCms branch June 21, 2018 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot find compilation library location for package 'Microsoft.Win32.Registry'
2 participants