DDRMenu - Remove Dependency on DotNetNuke.Web.Razor

[SkyeHoefling]

Part of: #2599

Description of problem

DotNetNuke.Modules.DDRMenu is dependent on DotNetNuke.Web.Razor which is marked for deletion in DNN 11.0 as part of the Razor Pages Pipeline Implementation.

The DDRMenu uses the RazorEngine which we can swap out to use the default Microsoft Implementation or Razor Pages.

public void Render(object source, HtmlTextWriter htmlWriter, TemplateDefinition liveDefinition)
{
    if (!(string.IsNullOrEmpty(liveDefinition.TemplateVirtualPath)))
    {
        var resolver = new PathResolver(liveDefinition.Folder);
        dynamic model = new ExpandoObject();
        model.Source = source;
        model.ControlID = DNNContext.Current.HostControl.ClientID;
        model.Options = ConvertToJson(liveDefinition.ClientOptions);
        model.DNNPath = resolver.Resolve("/", PathResolver.RelativeTo.Dnn);
        model.ManifestPath = resolver.Resolve("/", PathResolver.RelativeTo.Manifest);
        model.PortalPath = resolver.Resolve("/", PathResolver.RelativeTo.Portal);
        model.SkinPath = resolver.Resolve("/", PathResolver.RelativeTo.Skin);
        var modelDictionary = model as IDictionary<string, object>;
        liveDefinition.TemplateArguments.ForEach(a => modelDictionary.Add(a.Name, a.Value));

        var razorEngine = new RazorEngine(liveDefinition.TemplateVirtualPath, null, null);
        var writer = new StringWriter();
        razorEngine.Render<dynamic>(writer, model);

        htmlWriter.Write(writer.ToString());
    }
}

The line that is using DotNetNuke.Web.Razor is:

var razorEngine = new RazorEngine(liveDefinition.TemplateVirtualPath, null, null);

Description of solution

Ideally the new implementation of this code will use Microsoft's System.Web.Razor. If that does not work we will use the new Razor Pages Modules

[mitchelsellers]

DO we believe this is possible to support for a 10.0 release? I assume some breaking changes could be experienced?

[SkyeHoefling]

@mitchelsellers

DO we believe this is possible to support for a 10.0 release?

It is possible in my opinion to remove the Dependency on DotNetNuke.Web.Razor but the problem will be all modules in the wild using DDRMenu's Razor Templates. We would need to require those to update as the assembly is referenced right in the razor template. If you are unfamiliar with how DDRMenu Razor Templates work, take a look at the docs

Here is an example of what a DDRMenu Razor Template looks like:

@using DotNetNuke.Web.DDRMenu;
@using System.Dynamic; 
@inherits DotNetNuke.Web.Razor.DotNetNukeWebPage<dynamic>

@{ var root = Model.Source.root; }

@helper RenderNodes(IList<MenuNode> nodes) {
  if (nodes.Count > 0) {
    <ul>
      @foreach (var node in nodes) 
      {
        var cssClasses = new List<string>();
        if (node.First) { cssClasses.Add("first"); }
        if (node.Last) { cssClasses.Add("last"); }
        if (node.Selected) { cssClasses.Add("selected"); }
        
        var classString = new HtmlString((cssClasses.Count == 0) ? 
          "" : (" class=\"" + String.Join(" ", cssClasses.ToArray()) + "\""));
        
        <li @classString>
          @if (node.Enabled) 
          {
            <a href="@node.Url">@node.Text</a>
          } 
          else 
          {
            @node.Text
          }
          
          @RenderNodes(node.Children)
        </li>
      }
    </ul>
  }
}

@RenderNodes(root.Children)

At the top of the file it calls DotNetNuke.Web.Razor directly

@inherits DotNetNuke.Web.Razor.DotNetNukeWebPage<dynamic>

In my proposed solution we would require this line to be updated to use the new Razor Pages model which may greatly simplify this. There really are still a lot of unknowns to guarantee anything with Razor Pages.

However, I believe we can leverage existing RazorEngine from Microsoft assemblies. What we know as the RazorEngine in DNN is really just a wrapper for DotNetNuke.Web.Razor.DotNetNukeWebPage. See snippet below:

public abstract class DotNetNukeWebPage<TModel> :DotNetNukeWebPage where TModel : class
{
    private TModel _model;

    public new TModel Model
    {
        get { return _model ?? (_model = PageContext.Model as TModel); }
        set { _model = value; }
    }
}

The RazorEngine has a Render method that calls the underlying System.Web.WebPages implementation. This is a feature that is supported out of the box in Microsoft assemblies. We really should be able to include a patch for this module for DNN 10.0.0 that removes the Razor Engine and uses the underlying System.Web.WebPages implementation.

My concern with this patch is it will still contain a breaking change as the developers will need to update namespaces and such.

My Proposal

I do not think we patch this module for 9.x.x or 10.x.x. I recommend that once DNN 10.0.0 is released and Razor Pages is Generally Available, we migrate the API from DotNetNuke.Web.Razor -> Razor Pages and make any updates that need to be done at that time.

This proposal would result in only 1 breaking change which would occur at 11.0.0 which is what is being promised in #2618

[SkyeHoefling]

I created #2637 which will cover backwards compatibility for DDRMenu and provide developers a soft migration path away from DotNetNuke.Web.Razor

[SkyeHoefling]

When it is time to fully remove the dependency of DotNetNuke.Web.Razor we need to update the code change from #2637 to throw an exception that the template must inherit from System.Web.WebPages.WebPage

[valadas]

So should this issue be in the 10 milestone or 11 milestone ? Just trying to get all possible issues into a milestone :)

[mitchelsellers]

I put this as 11.0 for now

[SkyeHoefling]

@mitchelsellers if we get Razor Pages in for v10, full-feature and no feature flag could we try to get this in for v10? I'm not making any promises as I plan to PR Razor Pages behind a Feature Flag, but if are comfortable going all the way with it, we should make sure goes at the same time.

[valadas]

@ahoefling milestones are not set in stone, I am just trying to get every issue into one milestone to be able to vaguely scope the amount of work, no engagement :) Putting it hopefully for 10, but it can change.