JsComponent: improve error reporting and doccomments

partially adresses #1802

src/Framework/Framework/Controls/JsComponent.cs

@@ -1,9 +1,14 @@
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using DotVVM.Framework.Binding;
 using DotVVM.Framework.Binding.Expressions;
+using DotVVM.Framework.Binding.Properties;
 using DotVVM.Framework.Compilation.ControlTree;
 using DotVVM.Framework.Compilation.ControlTree.Resolved;
+using DotVVM.Framework.Compilation.Parser.Dothtml.Parser;
 using DotVVM.Framework.Compilation.Styles;
+using DotVVM.Framework.Compilation.Validation;
 using DotVVM.Framework.Hosting;
 using DotVVM.Framework.ResourceManagement;
 using DotVVM.Framework.Utils;
@@ -13,7 +18,8 @@ namespace DotVVM.Framework.Controls
     /// <summary> Control which initializes a client-side component. </summary>
     public class JsComponent : DotvvmControl
     {
-        /// <summary> If set to true, only globally registered JsComponents will be considered for rendering client-side. </summary>
+        /// <summary> If set to true, view modules are ignored and JsComponents registered using <c>dotvvm.registerGlobalComponent</c> will be considered for client-side rendering. </summary>
+        [MarkupOptions(AllowBinding = false)]
         public bool Global
         {
             get { return (bool)GetValue(GlobalProperty)!; }
@@ -23,7 +29,7 @@ public bool Global
             DotvvmProperty.Register<bool, JsComponent>(nameof(Global));
 
         /// <summary> Name by which the client-side component was registered. The name is case sensitive. </summary>
-        [MarkupOptions(Required = true)]
+        [MarkupOptions(Required = true, AllowBinding = false)]
         public string Name
         {
             get { return (string)GetValue(NameProperty)!; }
@@ -33,6 +39,7 @@ public string Name
             DotvvmProperty.Register<string, JsComponent>(nameof(Name));
 
         /// <summary> The JsComponent must have a wrapper HTML tag, this property configures which tag is used. By default, `div` is used. </summary>
+        [MarkupOptions(AllowBinding = false)]
         public string WrapperTagName
         {
             get { return (string)GetValue(WrapperTagNameProperty)!; }
@@ -132,7 +139,8 @@ protected override void RenderContents(IHtmlWriter writer, IDotvvmRequestContext
                 { "props", props },
                 { "templates", templates },
             };
-            if (GetValue(Internal.ReferencedViewModuleInfoProperty) is ViewModuleReferenceInfo viewModule)
+            if (GetValue(Internal.ReferencedViewModuleInfoProperty) is ViewModuleReferenceInfo viewModule &&
+                GetValue(GlobalProperty) is not true)
                 binding.Add("view", ViewModuleHelpers.GetViewIdJsExpression(viewModule, this));
 
             writer.AddKnockoutDataBind("dotvvm-js-component", binding);
@@ -154,5 +162,20 @@ public static void AddReferencedViewModuleInfoProperty(ResolvedControl control)
                 control.ConstructorParameters = null;
             }
         }
+
+        [ControlUsageValidator]
+        public static IEnumerable<ControlUsageError> ValidateUsage(ResolvedControl control)
+        {
+            if (!control.TreeRoot.HasProperty(Internal.ReferencedViewModuleInfoProperty) &&
+                control.GetProperty(GlobalProperty) is null or ResolvedPropertyValue { Value: false })
+            {
+                yield return new ControlUsageError(
+                    $"This view does not have any view modules registred, only global JsComponent will work. Add the `Global` property to this component, to make the intent clear.",
+                    DiagnosticSeverity.Warning,
+                    (control.DothtmlNode as DothtmlElementNode)?.TagNameNode
+                );
+            }
+        }
+
     }
 }

src/Framework/Framework/Resources/Scripts/viewModules/viewModuleManager.ts

@@ -131,12 +131,23 @@ export function findComponent(
     }
     if (name in globalComponent)
         return [null, globalComponent[name]]
-    throw Error("can not find control " + name)
+    if (compileConstants.debug) {
+        if (viewIdOrElement == null) {
+            throw Error(`Cannot find a global JsComponent ${name}. Make sure it is registred using dotvvm.registerGlobalComponent(${JSON.stringify(name)}, ...)`)
+        } else {
+            const moduleNames = [...getModules(viewIdOrElement)].map(m => m.moduleName)
+            throw Error(`Cannot find a JsComponent ${name} in modules ${moduleNames.join(", ")}, or the global registry. Make sure it is exported as $controls: { ${JSON.stringify(name)}: ... }`)
+        }
+    }
+    else {
+        throw Error("Cannot find JsComponent " + name)
+    }
 }
 
+/** Registers a component to be used in any JsComponent, independent of view modules */
 export function registerGlobalComponent(name: string, c: DotvvmJsComponentFactory) {
     if (name in globalComponent)
-        throw new Error(`Component ${name} is already registered`)
+        throw new Error(`JsComponent ${name} is already registered`)
     globalComponent[name] = c
 }
 

src/Tests/Runtime/ControlTree/DefaultControlTreeResolver/CompilationWarningsTests.cs

@@ -37,6 +37,32 @@ public void CompilationWarning_ValidationTargetPrimitiveType()
             Assert.AreEqual("BoolProp", warnings[0].tokens.Trim());
         }
 
+        [TestMethod]
+        public void CompilationWarning_JsComponentNoModules()
+        {
+            var warnings = GetWarnings($$"""
+                @viewModel {{typeof(TestViewModel)}}
+                <js:Test />
+            """);
+            Assert.AreEqual(1, warnings.Length);
+            StringAssert.Contains(warnings[0].warning, "This view does not have any view modules registred");
+            Assert.AreEqual("Test", warnings[0].tokens.Trim());
+        }
+
+        [TestMethod]
+        public void CompilationWarning_JsComponentFine()
+        {
+            XAssert.Empty(GetWarnings($$"""
+                @viewModel {{typeof(TestViewModel)}}
+                @js dotvvm.internal
+                <js:Test />
+            """));
+            XAssert.Empty(GetWarnings($$"""
+                @viewModel {{typeof(TestViewModel)}}
+                <js:Test Global />
+            """));
+        }
+
         [DataTestMethod]
         [DataRow("TestViewModel2")]
         [DataRow("VMArray")]
@@ -51,7 +77,7 @@ public void CompilationWarning_ValidationTargetPrimitiveType_Negative(string pro
         }
 
         [TestMethod]
-        public void DefaultViewCompiler_NonExistenPropertyWarning()
+        public void DefaultViewCompiler_NonExistentPropertyWarning()
         {
            var markup = $@"
 @viewModel System.Boolean
@@ -72,7 +98,7 @@ @viewModel System.Boolean
         }
 
         [TestMethod]
-        public void DefaultViewCompiler_NonExistenPropertyWarning_PrefixedGroup()
+        public void DefaultViewCompiler_NonExistentPropertyWarning_PrefixedGroup()
         {
            var markup = $@"
 @viewModel System.Boolean
@@ -90,6 +116,30 @@ @viewModel System.Boolean
             Assert.AreEqual("HTML attribute name 'IncludeInPage' should not contain uppercase letters. Did you intent to use a DotVVM property instead?", XAssert.Single(attribute2.AttributeNameNode.NodeWarnings));
         }
 
+        [TestMethod]
+        public void DefaultViewCompiler_NonExistentPropertyWarning_InnerElement()
+        {
+           var markup = $@"
+@viewModel System.Boolean
+<dot:Repeater>
+    <SepratrorTemplate>
+    </SepratrorTemplate>
+    test
+</dot:Repeater>
+";
+            var repeater = ParseSource(markup)
+                .Content.SelectRecursively(c => c.Content)
+                .Single(c => c.Metadata.Type == typeof(HierarchyRepeater));
+
+            var elementNode = (DothtmlElementNode)repeater.DothtmlNode;
+            var attribute1 = elementNode.Attributes.Single(a => a.AttributeName == "ItemClass");
+            var attribute2 = elementNode.Attributes.Single(a => a.AttributeName == "ItemIncludeInPage");
+
+            Assert.AreEqual(0, attribute1.AttributeNameNode.NodeWarnings.Count(), attribute1.AttributeNameNode.NodeWarnings.StringJoin(", "));
+            Assert.AreEqual("HTML attribute name 'IncludeInPage' should not contain uppercase letters. Did you intent to use a DotVVM property instead?", XAssert.Single(attribute2.AttributeNameNode.NodeWarnings));
+        }
+
+
         [TestMethod]
         public void DefaultViewCompiler_UnsupportedCallSite_ResourceBinding_Warning()
         {

src/Tests/Runtime/config-tests/ConfigurationSerializationTests.SerializeDefaultConfig.json

@@ -1129,7 +1129,8 @@
     "DotVVM.Framework.Controls.JsComponent": {
       "Global": {
         "type": "System.Boolean",
-        "defaultValue": false
+        "defaultValue": false,
+        "onlyHardcoded": true
       },
       "Html:ID": {
         "type": "System.String",
@@ -1142,11 +1143,13 @@
       },
       "Name": {
         "type": "System.String",
-        "required": true
+        "required": true,
+        "onlyHardcoded": true
       },
       "WrapperTagName": {
         "type": "System.String",
-        "defaultValue": "div"
+        "defaultValue": "div",
+        "onlyHardcoded": true
       }
     },
     "DotVVM.Framework.Controls.Label": {