Bug 1692157 - Update Javascript templates and provide Typescript template (#288)

Beatriz Rizental · web-flow · commit a74a6c16c8ef · 2021-02-23T16:30:16.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@
 
 - Add parser and object model support for `rate` metric type. ([bug 1645166](https://bugzilla.mozilla.org/show_bug.cgi?id=1645166))
 - Add parser and object model support for telemetry_mirror property. ([bug 1685406](https://bugzilla.mozilla.org/show_bug.cgi?id=1685406))
+- Update the Javascript template to match Glean.js expectations. ([bug 1693516](https://bugzilla.mozilla.org/show_bug.cgi?id=1693516))
+  - Glean.js has updated it's export strategy. It will now export each metric type as an independent module;
+  - Glean.js has dropped support for non ES6 modules.
+- Add support for generating Typescript code. ([bug 1692157](https://bugzilla.mozilla.org/show_bug.cgi?id=1692157))
+  - The templates added generate metrics and pings code for Glean.js.
 
 ## 2.4.0 (2021-02-18)
 
diff --git a/glean_parser/javascript.py b/glean_parser/javascript.py
@@ -41,7 +41,19 @@ def class_name(obj_type: str) -> str:
     else:
         class_name = util.Camelize(obj_type) + "MetricType"
 
-    return "Glean._private." + class_name
+    return class_name
+
+
+def import_path(obj_type: str) -> str:
+    """
+    Returns the import path of the given object inside the @mozilla/glean package.
+    """
+    if obj_type == "ping":
+        import_path = "ping"
+    else:
+        import_path = "metrics/" + util.camelize(obj_type)
+
+    return import_path
 
 
 def args(obj_type: str) -> Dict[str, object]:
@@ -54,12 +66,16 @@ def args(obj_type: str) -> Dict[str, object]:
     return {"common": util.common_metric_args, "extra": util.extra_metric_args}
 
 
-def output_javascript(
-    objs: metrics.ObjectTree, output_dir: Path, options: Optional[Dict[str, Any]] = None
+def output(
+    lang: str,
+    objs: metrics.ObjectTree,
+    output_dir: Path,
+    options: Optional[Dict[str, Any]] = None,
 ) -> None:
     """
-    Given a tree of objects, output Javascript code to `output_dir`.
+    Given a tree of objects, output Javascript or Typescript code to `output_dir`.
 
+    :param lang: Either "javascript" or "typescript";
     :param objects: A tree of objects (metrics and pings) as returned from
         `parser.parse_objects`.
     :param output_dir: Path to an output directory to write to.
@@ -82,15 +98,18 @@ def output_javascript(
         "javascript.jinja2",
         filters=(
             ("class_name", class_name),
+            ("import_path", import_path),
             ("js", javascript_datatypes_filter),
             ("args", args),
         ),
     )
 
     for category_key, category_val in objs.items():
-        filename = util.camelize(category_key) + ".js"
+        extension = ".js" if lang == "javascript" else ".ts"
+        filename = util.camelize(category_key) + extension
         filepath = output_dir / filename
 
+        types = set([util.camelize(obj.type) for obj in category_val.values()])
         with filepath.open("w", encoding="utf-8") as fd:
             fd.write(
                 template.render(
@@ -99,7 +118,56 @@ def output_javascript(
                     extra_args=util.extra_args,
                     namespace=namespace,
                     glean_namespace=glean_namespace,
+                    types=types,
+                    lang=lang,
                 )
             )
             # Jinja2 squashes the final newline, so we explicitly add it
             fd.write("\n")
+
+
+def output_javascript(
+    objs: metrics.ObjectTree, output_dir: Path, options: Optional[Dict[str, Any]] = None
+) -> None:
+    """
+    Given a tree of objects, output Javascript code to `output_dir`.
+
+    :param objects: A tree of objects (metrics and pings) as returned from
+        `parser.parse_objects`.
+    :param output_dir: Path to an output directory to write to.
+    :param options: options dictionary, with the following optional keys:
+
+        - `namespace`: The identifier of the global variable to assign to.
+                       This will only have and effect for Qt and static web sites.
+                       Default is `gleanAssets`.
+        - `glean_namespace`: Which version of the `@mozilla/glean` package to import,
+                             options are `webext` or `qt`. Default is `webext`.
+    """
+
+    output("javascript", objs, output_dir, options)
+
+
+def output_typescript(
+    objs: metrics.ObjectTree, output_dir: Path, options: Optional[Dict[str, Any]] = None
+) -> None:
+    """
+    Given a tree of objects, output Typescript code to `output_dir`.
+
+    # Note
+
+    The only difference between the typescript and javascript templates,
+    currently is the file extension.
+
+    :param objects: A tree of objects (metrics and pings) as returned from
+        `parser.parse_objects`.
+    :param output_dir: Path to an output directory to write to.
+    :param options: options dictionary, with the following optional keys:
+
+        - `namespace`: The identifier of the global variable to assign to.
+                       This will only have and effect for Qt and static web sites.
+                       Default is `gleanAssets`.
+        - `glean_namespace`: Which version of the `@mozilla/glean` package to import,
+                             options are `webext` or `qt`. Default is `webext`.
+    """
+
+    output("typescript", objs, output_dir, options)
diff --git a/glean_parser/templates/javascript.jinja2 b/glean_parser/templates/javascript.jinja2
@@ -1,57 +1,33 @@
-{# The final Javascript code is autogenerated, but this
+{# The final Javascript/Typescript code is autogenerated, but this
 Jinja2 template is not. Please file bugs! #}
 {% macro obj_declaration(obj) %}
 new {{ obj.type|class_name }}({
-            {% for arg_name in (obj.type|args).common if obj[arg_name] is defined %}
-            {{ arg_name|camelize }}: {{ obj[arg_name]|js }},
-            {% endfor %}
-        }, {% for arg_name in (obj.type|args).extra if obj[arg_name] is defined %}{{ obj[arg_name]|js }}{% endfor %}){% endmacro %}
+    {% for arg_name in (obj.type|args).common if obj[arg_name] is defined %}
+    {{ arg_name|camelize }}: {{ obj[arg_name]|js }},
+    {% endfor %}
+}{% for arg_name in (obj.type|args).extra if obj[arg_name] is defined %}, {{ obj[arg_name]|js }}{% endfor %}){% endmacro %}
 /* eslint-disable */
 
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-"use strict";
-
 // AUTOGENERATED BY glean_parser.  DO NOT EDIT.
 
-// Global variable to use when in Qt/QML environments,
-// where do not have access to the global object (i.e. window);
-var {{ category_name|camelize }};
+{% for type in types %}
+import {{ type|class_name }} from "@mozilla/glean/{{ glean_namespace }}/private/{{ type|import_path }}";
+{% endfor %}
+{% if lang == "javascript" %}
+
+"use strict"
+{% endif %}
+
+{% for obj in objs.values() %}
+/**
+ * {{ obj.description|wordwrap() | replace("\n", "\n * ") }}
+ *
+ * Generated from `{{ obj.identifier() }}`.
+ */
+export const {{ obj.name|camelize }} = {{ obj_declaration(obj) }};
 
-// Universal Module Definition (UMD) template based on:
-// https://github.com/umdjs/umd/blob/master/templates/returnExports.js
-(function (root, factory) {
-    if (typeof define === "function" && define.amd) {
-        // AMD. Register as an anonymous module.
-        define(["@mozilla/glean/{{ glean_namespace }}"], factory);
-    } else if (typeof module === "object" && module.exports) {
-        // Node. Does not work with strict CommonJS, but
-        // only CommonJS-like environments that support module.exports, like Node.
-        module.exports = factory(require("@mozilla/glean/{{ glean_namespace }}"));
-    } else if (typeof root === "undefined") {
-        // In Qt/QML environments we can't change the global object from Javascript.
-        // We will simply assing to a global variable in this case.
-        {{ category_name|camelize }} = factory({{ glean_namespace }}.Glean);
-    } else {
-        // Browser globals (root is window)
-        if (!root["{{ namespace }}"]) {
-            root["{{ namespace }}"] = {};
-        }
-        root["{{ namespace }}"]["{{ category_name|camelize }}"] = factory(root.{{ glean_namespace }});
-    }
-})(typeof self !== "undefined" ? self : this, function(Glean) {
-    {% if objs %}
-    return {
-        {% for obj in objs.values() %}
-        /**
-        * {{ obj.description|wordwrap() | replace("\n", "\n        * ") }}
-        *
-        * Generated from `{{ obj.identifier() }}`.
-        */
-        {{ obj.name|camelize }}: {{ obj_declaration(obj) }},
-        {% endfor %}
-    };
-    {% endif %}
-});
+{% endfor %}
diff --git a/glean_parser/translate.py b/glean_parser/translate.py
@@ -54,6 +54,7 @@ def __init__(
 OUTPUTTERS = {
     "csharp": Outputter(csharp.output_csharp, ["*.cs"]),
     "javascript": Outputter(javascript.output_javascript, []),
+    "typescript": Outputter(javascript.output_typescript, []),
     "kotlin": Outputter(kotlin.output_kotlin, ["*.kt"]),
     "markdown": Outputter(markdown.output_markdown, []),
     "swift": Outputter(swift.output_swift, ["*.swift"]),
diff --git a/tests/test_javascript.py b/tests/test_javascript.py
@@ -14,7 +14,7 @@
 ROOT = Path(__file__).parent
 
 
-def test_parser(tmpdir):
+def test_parser_js(tmpdir):
     """Test translating metrics to Javascript files."""
     tmpdir = Path(str(tmpdir))
 
@@ -39,14 +39,56 @@ def test_parser(tmpdir):
     # Make sure descriptions made it in
     with (tmpdir / "corePing.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
+        assert "use strict" in content
         assert "True if the user has set Firefox as the default browser." in content
 
     with (tmpdir / "telemetry.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
+        assert "use strict" in content
         assert "جمع 搜集" in content
 
     with (tmpdir / "gleanInternalMetrics.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
+        assert "use strict" in content
+        assert 'category: ""' in content
+
+
+def test_parser_ts(tmpdir):
+    """Test translating metrics to Typescript files."""
+    tmpdir = Path(str(tmpdir))
+
+    translate.translate(
+        ROOT / "data" / "core.yaml",
+        "typescript",
+        tmpdir,
+        None,
+        {"allow_reserved": True},
+    )
+
+    assert set(x.name for x in tmpdir.iterdir()) == set(
+        [
+            "corePing.ts",
+            "telemetry.ts",
+            "environment.ts",
+            "dottedCategory.ts",
+            "gleanInternalMetrics.ts",
+        ]
+    )
+
+    # Make sure descriptions made it in
+    with (tmpdir / "corePing.ts").open("r", encoding="utf-8") as fd:
+        content = fd.read()
+        assert "use strict" not in content
+        assert "True if the user has set Firefox as the default browser." in content
+
+    with (tmpdir / "telemetry.ts").open("r", encoding="utf-8") as fd:
+        content = fd.read()
+        assert "use strict" not in content
+        assert "جمع 搜集" in content
+
+    with (tmpdir / "gleanInternalMetrics.ts").open("r", encoding="utf-8") as fd:
+        content = fd.read()
+        assert "use strict" not in content
         assert 'category: ""' in content
 
 
@@ -87,7 +129,42 @@ def test_metric_class_name():
         extra_keys={"my_extra": {"description": "an extra"}},
     )
 
-    assert javascript.class_name(event.type) == "Glean._private.EventMetricType"
+    assert javascript.class_name(event.type) == "EventMetricType"
+
+    boolean = metrics.Boolean(
+        type="boolean",
+        category="category",
+        name="metric",
+        bugs=["http://bugzilla.mozilla.com/12345"],
+        notification_emails=["nobody@example.com"],
+        description="description...",
+        expires="never",
+    )
+    assert javascript.class_name(boolean.type) == "BooleanMetricType"
+
+    ping = pings.Ping(
+        name="custom",
+        description="description...",
+        include_client_id=True,
+        bugs=["http://bugzilla.mozilla.com/12345"],
+        notification_emails=["nobody@nowhere.com"],
+    )
+    assert javascript.class_name(ping.type) == "PingType"
+
+
+def test_import_path():
+    event = metrics.Event(
+        type="event",
+        category="category",
+        name="metric",
+        bugs=["http://bugzilla.mozilla.com/12345"],
+        notification_emails=["nobody@example.com"],
+        description="description...",
+        expires="never",
+        extra_keys={"my_extra": {"description": "an extra"}},
+    )
+
+    assert javascript.import_path(event.type) == "metrics/event"
 
     boolean = metrics.Boolean(
         type="boolean",
@@ -98,7 +175,7 @@ def test_metric_class_name():
         description="description...",
         expires="never",
     )
-    assert javascript.class_name(boolean.type) == "Glean._private.BooleanMetricType"
+    assert javascript.import_path(boolean.type) == "metrics/boolean"
 
     ping = pings.Ping(
         name="custom",
@@ -107,7 +184,7 @@ def test_metric_class_name():
         bugs=["http://bugzilla.mozilla.com/12345"],
         notification_emails=["nobody@nowhere.com"],
     )
-    assert javascript.class_name(ping.type) == "Glean._private.PingType"
+    assert javascript.import_path(ping.type) == "ping"
 
 
 # TODO: Activate once Glean.js adds support for labeled metric types in Bug 1682573.
@@ -189,5 +266,5 @@ def test_arguments_are_generated_in_deterministic_order(tmpdir):
     with (tmpdir / "event.js").open("r", encoding="utf-8") as fd:
         content = fd.read()
         content = " ".join(content.split())
-        expected = 'new Glean._private.EventMetricType({ category: "event", name: "example", sendInPings: ["events"], lifetime: "ping", disabled: false, }, ["alice", "bob", "charlie"]),'  # noqa
+        expected = 'export const example = new EventMetricType({ category: "event", name: "example", sendInPings: ["events"], lifetime: "ping", disabled: false, }, ["alice", "bob", "charlie"]);'  # noqa
         assert expected in content
