Support ?? in observable bindings (#33)

# Pull Request

## Motivation

We are enabling a new eslint rule
(`@typescript-eslint/strict-boolean-expressions`) in Nimble that
disallows using non-booleans in boolean expressions. We have some
template bindings like `x.foo || 'default_val'` which are flagged by
this rule and would ideally be rewritten as `x.foo ?? 'default_val'`.
However, without proper support for `??` in binding expressions, we
would have to rewrite them as `x.foo !== undefined ? x.foo :
'default_value'`, which is excessively verbose.

## 📖 Description

See ni/nimble#2127

The observable binding logic uses a regex pattern to determine which
kinds of expressions in a binding should cause the binding to be marked
as volatile. The regex did not include `??`, presumably because the FAST
libraries targeted `es2015`, in which a binding like `(x) => x.trigger
?? x.value` would be transpiled to something like `(x) => { var _a;
return (_a = x.trigger) !== null && _a !== void 0 ? _a : x.value; }`.
However, a client library like Nimble, which targets a later version of
the language that directly supports `??` requires FAST to include `??`
in its regex.

As part of this change, I've:
- updated the regex to include `??`
- changed the target of the FAST libraries to `es2022` (for consistency
with our client libraries)
- fixed an unrelated issue with aggregate bindings never being marked as
volatile
- added test cases

Author: m-akinc

change/@ni-fast-element-081938dc-abe0-47de-a38c-ca7a64927067.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add support for ?? in observable bindings",
+  "packageName": "@ni/fast-element",
+  "email": "7282195+m-akinc@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

change/@ni-fast-foundation-00762393-037b-4486-a23e-9f13399d111d.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Target es2022 rather than es2015",
+  "packageName": "@ni/fast-foundation",
+  "email": "7282195+m-akinc@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

change/@ni-fast-react-wrapper-4225f7d3-967a-4677-bf96-8175e1a67415.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Target es2022 rather than es2015",
+  "packageName": "@ni/fast-react-wrapper",
+  "email": "7282195+m-akinc@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

packages/utilities/fast-react-wrapper/tsconfig.json

@@ -4,8 +4,9 @@
         "declarationDir": "dist/dts",
         "outDir": "dist/esm",
         "experimentalDecorators": true,
-        "target": "es2015",
-        "module": "ESNext",
+        "useDefineForClassFields": false,
+        "target": "es2022",
+        "module": "es2020",
         "importHelpers": true,
         "jsx": "react",
         "types": [
@@ -14,7 +15,7 @@
         ],
         "lib": [
             "DOM",
-            "ES2015"
+            "ES2022"
         ]
     },
     "include": ["src"]

packages/web-components/fast-element/src/observation/observable.spec.ts

@@ -9,6 +9,7 @@ describe("The Observable", () => {
         @observable child = new ChildModel();
         @observable child2 = new ChildModel();
         @observable trigger = 0;
+        @observable nullableTrigger: number | null = 0;
         @observable value = 10;
 
         childChangedCalled = false;
@@ -41,9 +42,14 @@ describe("The Observable", () => {
         }
 
         @volatile
-        get andCondition() {
+        get andConditional() {
             return this.trigger && this.value;
         }
+
+        @volatile
+        get nullishCoalescingConditional() {
+            return this.nullableTrigger ?? this.value;
+        }
     }
 
     class ChildModel {
@@ -121,18 +127,18 @@ describe("The Observable", () => {
         it("can list all accessors for an object", () => {
             const accessors = Observable.getAccessors(new Model());
 
-            expect(accessors.length).to.equal(4);
+            expect(accessors.length).to.equal(5);
             expect(accessors[0].name).to.equal("child");
             expect(accessors[1].name).to.equal("child2");
         });
 
         it("can list accessors for an object, including the prototype chain", () => {
             const accessors = Observable.getAccessors(new DerivedModel());
 
-            expect(accessors.length).to.equal(5);
+            expect(accessors.length).to.equal(6);
             expect(accessors[0].name).to.equal("child");
             expect(accessors[1].name).to.equal("child2");
-            expect(accessors[4].name).to.equal("derivedChild");
+            expect(accessors[5].name).to.equal("derivedChild");
         });
 
         it("can create a binding observer", () => {
@@ -463,7 +469,7 @@ describe("The Observable", () => {
         });
 
         it("notifies on changes in a computed && expression", async () => {
-            const binding = (x: Model) => x.trigger && x.value;
+            const binding = (x: Model) => x.andConditional;
 
             let wasNotified = false;
             const observer = Observable.binding(binding, {
@@ -497,6 +503,80 @@ describe("The Observable", () => {
             expect(value).to.equal(binding(model));
         });
 
+        it("notifies on changes in a ?? expression", async () => {
+            const binding = (x: Model) => x.nullableTrigger ?? x.value;
+
+            let wasNotified = false;
+            const observer = Observable.binding(binding, {
+                handleChange() {
+                    wasNotified = true;
+                },
+            });
+
+            const model = new Model();
+            model.nullableTrigger = 0;
+
+            let value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+
+            expect(wasNotified).to.be.false;
+            model.nullableTrigger = null;
+
+            await DOM.nextUpdate();
+
+            expect(wasNotified).to.be.true;
+
+            value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+
+            wasNotified = false;
+            model.value = 20; // nullableTrigger is null, so value change should notify
+
+            await DOM.nextUpdate();
+
+            expect(wasNotified).to.be.true;
+
+            value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+        });
+
+        it("notifies on changes in a computed ?? expression", async () => {
+            const binding = (x: Model) => x.nullishCoalescingConditional;
+
+            let wasNotified = false;
+            const observer = Observable.binding(binding, {
+                handleChange() {
+                    wasNotified = true;
+                },
+            });
+
+            const model = new Model();
+            model.nullableTrigger = 0;
+
+            let value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+
+            expect(wasNotified).to.be.false;
+            model.nullableTrigger = null;
+
+            await DOM.nextUpdate();
+
+            expect(wasNotified).to.be.true;
+
+            value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+
+            wasNotified = false;
+            model.value = 20; // nullableTrigger is null, so value change should notify
+
+            await DOM.nextUpdate();
+
+            expect(wasNotified).to.be.true;
+
+            value = observer.observe(model, defaultExecutionContext);
+            expect(value).to.equal(binding(model));
+        });
+
         it("notifies on changes in an || expression", async () => {
             const binding = (x: Model) => x.trigger || x.value;
 

packages/web-components/fast-element/src/observation/observable.ts

@@ -89,7 +89,7 @@ export interface BindingObserver<TSource = any, TReturn = any, TParent = any>
  * @public
  */
 export const Observable = FAST.getById(KernelServiceId.observable, () => {
-    const volatileRegex = /(:|&&|\|\||if)/;
+    const volatileRegex = /(\?\?|:|&&|\|\||if)/;
     const notifierLookup = new WeakMap<any, Notifier>();
     const queueUpdate = DOM.queueUpdate;
     let watcher: BindingObserverImplementation | undefined = void 0;

packages/web-components/fast-element/src/templating/binding.ts

@@ -197,10 +197,11 @@ export class HTMLBindingDirective extends TargetedHTMLDirective {
     /**
      * Creates an instance of BindingDirective.
      * @param binding - A binding that returns the data used to update the DOM.
+     * @param isVolatile - Optional parameter indicating whether the binding is volatile. If not provided, the volatility of the binding is determined by Observable.isVolatileBinding().
      */
-    public constructor(public binding: Binding) {
+    public constructor(public binding: Binding, isVolatile?: boolean) {
         super();
-        this.isBindingVolatile = Observable.isVolatileBinding(this.binding);
+        this.isBindingVolatile = isVolatile ?? Observable.isVolatileBinding(this.binding);
     }
 
     /**

packages/web-components/fast-element/src/templating/compiler.spec.ts

@@ -5,7 +5,7 @@ import { defaultExecutionContext } from "../observation/observable";
 import { css } from "../styles/css";
 import type { StyleTarget } from "../styles/element-styles";
 import { toHTML, uniqueElementName } from "../__test__/helpers";
-import { HTMLBindingDirective } from "./binding";
+import { BindingBehavior, HTMLBindingDirective } from "./binding";
 import { compileTemplate } from "./compiler";
 import type { HTMLDirective } from "./html-directive";
 import { html } from "./template";
@@ -25,6 +25,10 @@ describe("The template compiler", () => {
         return new HTMLBindingDirective(() => result);
     }
 
+    function volatileBinding() {
+        return new HTMLBindingDirective(() => Math.random() > 0.5 ? "greater" : "less");
+    }
+
     const scope = {};
 
     context("when compiling content", () => {
@@ -343,6 +347,42 @@ describe("The template compiler", () => {
                 }
             });
         });
+
+        it(`marks aggregate binding volatile when first binding is volatile and second is not`, () => {
+            const html = `<a href="beginning ${inline(0)} ${inline(1)}">Link</a>`;
+            const directives = [volatileBinding(), binding()];
+            const { fragment, viewBehaviorFactories } = compile(html, directives);
+
+            const behavior = viewBehaviorFactories[0].createBehavior(fragment) as BindingBehavior;
+            expect(behavior.isBindingVolatile).to.be.true;
+        });
+
+        it(`marks aggregate binding volatile when second binding is volatile and first is not`, () => {
+            const html = `<a href="beginning ${inline(0)} ${inline(1)}">Link</a>`;
+            const directives = [binding(), volatileBinding()];
+            const { fragment, viewBehaviorFactories } = compile(html, directives);
+
+            const behavior = viewBehaviorFactories[0].createBehavior(fragment) as BindingBehavior;
+            expect(behavior.isBindingVolatile).to.be.true;
+        });
+
+        it(`marks aggregate binding non-volatile when neither of two bindings are volatile`, () => {
+            const html = `<a href="beginning ${inline(0)} ${inline(1)}">Link</a>`;
+            const directives = [binding(), binding()];
+            const { fragment, viewBehaviorFactories } = compile(html, directives);
+
+            const behavior = viewBehaviorFactories[0].createBehavior(fragment) as BindingBehavior;
+            expect(behavior.isBindingVolatile).to.be.false;
+        });
+
+        it(`marks aggregate binding volatile when both of two bindings are volatile`, () => {
+            const html = `<a href="beginning ${inline(0)} ${inline(1)}">Link</a>`;
+            const directives = [volatileBinding(), volatileBinding()];
+            const { fragment, viewBehaviorFactories } = compile(html, directives);
+
+            const behavior = viewBehaviorFactories[0].createBehavior(fragment) as BindingBehavior;
+            expect(behavior.isBindingVolatile).to.be.true;
+        });
     });
 
     context("when compiling comments", () => {

packages/web-components/fast-element/src/templating/compiler.ts

@@ -1,5 +1,5 @@
 import { _interpolationEnd, _interpolationStart, DOM } from "../dom.js";
-import type { Binding, ExecutionContext } from "../observation/observable.js";
+import { type Binding, type ExecutionContext, Observable } from "../observation/observable.js";
 import { HTMLBindingDirective } from "./binding.js";
 import type { HTMLDirective, NodeBehaviorFactory } from "./html-directive.js";
 
@@ -53,13 +53,15 @@ function createAggregateBinding(
     }
 
     let targetName: string | undefined;
+    let isVolatile = false;
     const partCount = parts.length;
     const finalParts = parts.map((x: string | InlineDirective) => {
         if (typeof x === "string") {
             return (): string => x;
         }
 
         targetName = x.targetName || targetName;
+        isVolatile = isVolatile || Observable.isVolatileBinding(x.binding);
         return x.binding;
     });
 
@@ -73,7 +75,7 @@ function createAggregateBinding(
         return output;
     };
 
-    const directive = new HTMLBindingDirective(binding);
+    const directive = new HTMLBindingDirective(binding, isVolatile);
     directive.targetName = targetName;
     return directive;
 }

packages/web-components/fast-element/tsconfig.json

@@ -8,17 +8,17 @@
         "experimentalDecorators": true,
         "noImplicitAny": false,
         "strictPropertyInitialization": false,
-        "target": "es2015",
-        "module": "ESNext",
+        "useDefineForClassFields": false,
+        "target": "es2022",
+        "module": "es2020",
         "types": [
             "mocha",
             "webpack-env",
             "web-ie11"
         ],
         "lib": [
             "DOM",
-            "ES2015",
-            "ES2016.Array.Include"
+            "ES2022"
         ]
     },
     "include": ["src"]