From e8386b7b7764baafb8383134444555701a81538a Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Fri, 26 Jun 2026 11:25:56 -0400 Subject: [PATCH 1/9] getExpression is not an async function While here, I cleaned up all the jsdoc comments. We don't need to specify types in the comments since we're using TypeScript. --- src/types/global.d.ts | 2 +- src/ui/jexlParser.ts | 98 ++++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/src/types/global.d.ts b/src/types/global.d.ts index 7506a20..bfee9b9 100644 --- a/src/types/global.d.ts +++ b/src/types/global.d.ts @@ -155,7 +155,7 @@ declare namespace browser.experiments.nimbus { function setCollection(collectionId: string): Promise; - function evaluateJEXL(expression: string, context: object): Promise; + function evaluateJEXL(expression: string, context: object): Promise; function getClientContext(): Promise; diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index e3b1287..33ac112 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -21,9 +21,11 @@ const grammar = { /** * Evaluates a JEXL expression within a given context. - * @param {string} expression - The JEXL expression to evaluate. - * @param {object} context - The context in which to evaluate the expression. - * @returns {Promise} - The result of the evaluation as a string. + * + * @param expression The JEXL expression to evaluate. + * @param context The context in which to evaluate the expression. + * + * @returns The result of the evaluation as a string. */ export async function evaluateJexl( expression: string, @@ -51,14 +53,16 @@ export async function evaluateJexl( /** * Evaluates a JEXL expression using the browser's Nimbus API. - * @param {string} expression - The JEXL expression to evaluate. - * @param {object} context - The context in which to evaluate the expression. - * @returns {Promise} - The result of the evaluation. + * + * @param expression - The JEXL expression to evaluate. + * @param context - The context in which to evaluate the expression. + * + * @returns The result of the evaluation. */ async function evaluateExpression( expression: string, context: object, -): Promise { +): Promise { try { return await browser.experiments.nimbus.evaluateJEXL(expression, context); } catch (error) { @@ -69,10 +73,12 @@ async function evaluateExpression( /** * Traverses the AST to evaluate sub-expressions and collect parts that evaluate to false. - * @param {ASTNode} ast - The AST node to traverse. - * @param {object} context - The context in which to evaluate the expressions. - * @param {string[]} falseParts - An array to collect expressions that evaluate to false. - * @returns {Promise} - The result of the traversal. + * + * @param ast The AST node to traverse. + * @param context The context in which to evaluate the expressions. + * @param falseParts An array to collect expressions that evaluate to false. + * + * @returns The result of the traversal. */ async function traverseAst( ast: ASTNode, @@ -83,16 +89,16 @@ async function traverseAst( return true; } - const subExpr = await getExpression(ast); + const subExpr = getExpression(ast); const result = await evaluateExpression(subExpr, context); if (ast.type === "BinaryExpression" || ast.type === "LogicalExpression") { const leftResult = await evaluateExpression( - await getExpression(ast.left), + getExpression(ast.left), context, ); const rightResult = await evaluateExpression( - await getExpression(ast.right), + getExpression(ast.right), context, ); @@ -164,8 +170,10 @@ async function traverseAst( /** * Retrieves the full identifier string from an AST node. - * @param {Identifier} ast - The AST node representing an identifier. - * @returns {string} - The full identifier string. + * + * @param ast The AST node representing an identifier. + * + * @returns The full identifier string. */ function getFullIdentifier(ast: Identifier): string { if (!ast.from) { @@ -176,17 +184,19 @@ function getFullIdentifier(ast: Identifier): string { /** * Converts an AST node to its corresponding JEXL expression string. - * @param {ASTNode} ast - The AST node to convert. - * @returns {Promise} - The JEXL expression string. + * + * @param ast The AST node to convert. + * + * @returns The JEXL expression string. */ -async function getExpression(ast: ASTNode): Promise { +function getExpression(ast: ASTNode): string { if (!ast) { return ""; } if (ast.type === "BinaryExpression" || ast.type === "LogicalExpression") { - const leftExpr = await getExpression(ast.left); - const rightExpr = await getExpression(ast.right); + const leftExpr = getExpression(ast.left); + const rightExpr = getExpression(ast.right); const leftWrapped = ast.left.type === "Literal" || ast.left.type === "Identifier" || @@ -201,47 +211,41 @@ async function getExpression(ast: ASTNode): Promise { : `(${rightExpr})`; return `${leftWrapped} ${ast.operator} ${rightWrapped}`; } else if (ast.type === "UnaryExpression") { - const rightExpr = await getExpression(ast.right); + const rightExpr = getExpression(ast.right); return `${ast.operator}(${rightExpr})`; } else if (ast.type === "Transform") { - const subjectExpr = await getExpression(ast.subject); - const argsExpr = ast.args - ? (await Promise.all(ast.args.map(getExpression))).join(", ") - : ""; + const subjectExpr = getExpression(ast.subject); + const argsExpr = ast.args ? ast.args.map(getExpression).join(", ") : ""; return argsExpr.length === 0 ? `${subjectExpr}|${ast.name}` : `${subjectExpr}|${ast.name}(${argsExpr})`; } else if (ast.type === "FilterExpression") { - const subjectExpr = await getExpression(ast.subject); - const filterExpr = await getExpression(ast.expr); + const subjectExpr = getExpression(ast.subject); + const filterExpr = getExpression(ast.expr); return `${subjectExpr}[${filterExpr}]`; } else if (ast.type === "Literal") { return typeof ast.value === "string" ? `'${ast.value}'` : `${ast.value}`; } else if (ast.type === "Identifier") { return getFullIdentifier(ast); } else if (ast.type === "ArrayLiteral") { - const elementsExpr = (await Promise.all(ast.value.map(getExpression))).join( - ", ", - ); + const elementsExpr = ast.value.map(getExpression).join(", "); return `[${elementsExpr}]`; } else if (ast.type === "ObjectLiteral") { const entries = Object.entries(ast.value); - const objectExpr = await Promise.all( - entries.map(async ([key, value]) => { - const valueExpr = await getExpression(value); - if (value.type === "ObjectLiteral") { - const nestedEntries = Object.entries(value.value); - const nestedObjectExpr = await Promise.all( - nestedEntries.map(async ([nestedKey, nestedValue]) => { - const nestedValueExpr = await getExpression(nestedValue); - return `${nestedKey}: ${nestedValueExpr}`; - }), - ); - return `${key}: { ${nestedObjectExpr.join(", ")} }`; - } - return `${key}: ${valueExpr}`; - }), - ); + const objectExpr = entries.map(async ([key, value]) => { + const valueExpr = getExpression(value); + if (value.type === "ObjectLiteral") { + const nestedEntries = Object.entries(value.value); + const nestedObjectExpr = nestedEntries.map( + ([nestedKey, nestedValue]) => { + const nestedValueExpr = getExpression(nestedValue); + return `${nestedKey}: ${nestedValueExpr}`; + }, + ); + return `${key}: { ${nestedObjectExpr.join(", ")} }`; + } + return `${key}: ${valueExpr}`; + }); return `{ ${objectExpr.join(", ")} }`; } return ""; From 8f3dbd9b4e12dc81cf76f8ccac924a2f6752ea87 Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Fri, 26 Jun 2026 14:05:50 -0400 Subject: [PATCH 2/9] Support property accessors on object and array literals When trying to re-serialize an identifier AST, the code assumed that the only possible LHS of a property access was a literal. However, mozjexl supports accessors on object literals and on array literals (in which case the attribute is accessed on the first element, or undefined is returned). This allows for parsing expressions of the form: * `{x: 1}.x` * `[{x: 1}].x` Fixes #224 --- src/types/global.d.ts | 2 +- src/ui/jexlParser.ts | 25 +++++++------------------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/types/global.d.ts b/src/types/global.d.ts index bfee9b9..3f64529 100644 --- a/src/types/global.d.ts +++ b/src/types/global.d.ts @@ -75,7 +75,7 @@ declare module "mozjexl/lib/parser/Parser" { type Identifier = { type: "Identifier"; value: string; - from?: Identifier; + from?: ASTNode; }; type ArrayLiteral = { diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index 33ac112..34eff53 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -1,6 +1,6 @@ import { elements as baseGrammar } from "mozjexl/lib/grammar"; import Lexer from "mozjexl/lib/Lexer"; -import Parser, { ASTNode, Identifier } from "mozjexl/lib/parser/Parser"; +import Parser, { ASTNode } from "mozjexl/lib/parser/Parser"; const grammar = { ...baseGrammar, @@ -114,7 +114,7 @@ async function traverseAst( } } else if (ast.type === "UnaryExpression") { const rightResult = await evaluateExpression( - await getExpression(ast.right), + getExpression(ast.right), context, ); if (result === false && rightResult !== false) { @@ -168,20 +168,6 @@ async function traverseAst( return result; } -/** - * Retrieves the full identifier string from an AST node. - * - * @param ast The AST node representing an identifier. - * - * @returns The full identifier string. - */ -function getFullIdentifier(ast: Identifier): string { - if (!ast.from) { - return ast.value; - } - return `${getFullIdentifier(ast.from)}.${ast.value}`; -} - /** * Converts an AST node to its corresponding JEXL expression string. * @@ -226,13 +212,16 @@ function getExpression(ast: ASTNode): string { } else if (ast.type === "Literal") { return typeof ast.value === "string" ? `'${ast.value}'` : `${ast.value}`; } else if (ast.type === "Identifier") { - return getFullIdentifier(ast); + if (ast.from) { + return getExpression(ast.from) + "." + ast.value; + } + return ast.value; } else if (ast.type === "ArrayLiteral") { const elementsExpr = ast.value.map(getExpression).join(", "); return `[${elementsExpr}]`; } else if (ast.type === "ObjectLiteral") { const entries = Object.entries(ast.value); - const objectExpr = entries.map(async ([key, value]) => { + const objectExpr = entries.map(([key, value]) => { const valueExpr = getExpression(value); if (value.type === "ObjectLiteral") { const nestedEntries = Object.entries(value.value); From e57974fe65d04707730313feb0ecd4d9afeb1535 Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Fri, 26 Jun 2026 14:27:06 -0400 Subject: [PATCH 3/9] Simplify getExpression() for ObjectLiterals The serialization code for ObjectLiterals had a special case to handle nested literals. However, that case is unnecessary. This greatly simplifies the case. --- src/ui/jexlParser.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index 34eff53..33b2f25 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -223,16 +223,6 @@ function getExpression(ast: ASTNode): string { const entries = Object.entries(ast.value); const objectExpr = entries.map(([key, value]) => { const valueExpr = getExpression(value); - if (value.type === "ObjectLiteral") { - const nestedEntries = Object.entries(value.value); - const nestedObjectExpr = nestedEntries.map( - ([nestedKey, nestedValue]) => { - const nestedValueExpr = getExpression(nestedValue); - return `${nestedKey}: ${nestedValueExpr}`; - }, - ); - return `${key}: { ${nestedObjectExpr.join(", ")} }`; - } return `${key}: ${valueExpr}`; }); return `{ ${objectExpr.join(", ")} }`; From 2bd711ba73d5f92471552a723c489ae14a6f006f Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Sat, 27 Jun 2026 18:06:19 -0400 Subject: [PATCH 4/9] Fix string re-quoting in jexlParser Our string quoting algorithm was not particularily advanced -- it simply wrapped the result in string in single quotes. We now are using a more thorough algorithm that attempts to quote the string with any quote not used in the string and falls back to escaping single/double quotes (where appropriate). We also use this same algorithm when returning string values from `evaluateJexl()` so that the result can be used in the debugger verbatim. Fixes #228 --- src/ui/jexlParser.ts | 54 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index 33b2f25..fa5efc6 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -45,10 +45,14 @@ export async function evaluateJexl( await traverseAst(ast, context, falseParts); const finalResult = await evaluateExpression(expression, context); - const falsePartsStr = !finalResult - ? `\n\nFalse Parts:\n${falseParts.join("\n")}` - : ""; - return `${JSON.stringify(finalResult, null, 2)}${falsePartsStr}`; + const finalResultStr = + typeof finalResult === "string" + ? quoteString(finalResult) + : JSON.stringify(finalResult, null, 2); + + const falsePartsStr = + finalResult === false ? `\n\nFalse Parts:\n${falseParts.join("\n")}` : ""; + return `${finalResultStr}${falsePartsStr}`; } /** @@ -66,7 +70,10 @@ async function evaluateExpression( try { return await browser.experiments.nimbus.evaluateJEXL(expression, context); } catch (error) { - console.error(`Error evaluating part "${expression}":`, error); + console.error( + `Error evaluating expression ${quoteString(expression)}:`, + error, + ); throw error; } } @@ -210,7 +217,9 @@ function getExpression(ast: ASTNode): string { const filterExpr = getExpression(ast.expr); return `${subjectExpr}[${filterExpr}]`; } else if (ast.type === "Literal") { - return typeof ast.value === "string" ? `'${ast.value}'` : `${ast.value}`; + return typeof ast.value === "string" + ? quoteString(ast.value) + : `${ast.value}`; } else if (ast.type === "Identifier") { if (ast.from) { return getExpression(ast.from) + "." + ast.value; @@ -229,3 +238,36 @@ function getExpression(ast: ASTNode): string { } return ""; } + +/** + * Quote a string so that it is re-parseable by mozjexl. + * + * @param s The string to quote. + * + * @returns A quoted version of the given string. + */ +function quoteString(s: string) { + // If the string does not include both types of quotes, it is easy to re-quote. + if (!s.includes(`"`)) { + return `"${s}"`; + } + + if (!s.includes(`'`)) { + return `'${s}'`; + } + + // However, if it does contain both types of quotes, then we must pick one and + // escape every unescaped instance of that quote inside `s`. + // + // A quote is unescaped if it is preceded by an even number of backslashes + // (including 0). + + // The mozjexl lexer does not accept a string of the form `"\""`, so we need + // to escape all the single quotes within s and surround it with single + // quotes. + if (s.endsWith(`"`)) { + return `'` + s.replaceAll(/(\\\\)*'/g, `$1\\'`) + `'`; + } + + return `"` + s.replaceAll(/(\\\\)*"/g, `$1\\"`) + `"`; +} From e9d5aebc7a5e773bb9298abeecab9afb7975594b Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Sat, 27 Jun 2026 18:14:48 -0400 Subject: [PATCH 5/9] Evaluate expressions before attempting to debug them If the expression is invalid, we can rely on evaluateExpression to throw and avoid doing the work to construct the AST (again) and recursively debug it. This also results in the errors from the evaluator instead of the parser. --- src/ui/jexlParser.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index fa5efc6..bd2c706 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -35,6 +35,8 @@ export async function evaluateJexl( throw new Error("Empty expression"); } + const finalResult = await evaluateExpression(expression, context); + const lexer = new Lexer(grammar); const parser = new Parser(grammar); @@ -44,7 +46,6 @@ export async function evaluateJexl( await traverseAst(ast, context, falseParts); - const finalResult = await evaluateExpression(expression, context); const finalResultStr = typeof finalResult === "string" ? quoteString(finalResult) From f9ea40ad0622966bed96d8222af4d46c279042b3 Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Wed, 24 Jun 2026 14:59:46 -0400 Subject: [PATCH 6/9] Make the JEXL Debugger Page more visually consistent Margins and padding have been tweaked to make everything line up with the fewest additional margin and padding classes. The `.large-checkbox` class was previously using `transform: scale(1.5)` to make the checkbox larger. However, `transform: scale(...)` is tricky as it also affects margin and padding. It ended up being simpler to change it to use constant height and width to make it larger and also aligned to the rest of the elements. Textareas (for object values) and text inputs (for strings) now take up the entire page width, which helps for editing bigger JSON payloads. All context inputs are now connected to their labels via DOM IDs, which means that clicking on the attribute name will focus the inputs. Field labels are now formatted monospace to increase readability. --- src/ui/components/JEXLDebuggerPage.tsx | 33 +++++++++++++++++++------- src/ui/index.css | 3 ++- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/ui/components/JEXLDebuggerPage.tsx b/src/ui/components/JEXLDebuggerPage.tsx index af69b29..627c065 100644 --- a/src/ui/components/JEXLDebuggerPage.tsx +++ b/src/ui/components/JEXLDebuggerPage.tsx @@ -72,6 +72,7 @@ function ContextField({ value={value as string} onChange={handleChange} className="p-3 w-50 grey-border short-text" + id={`context.${contextKey}`} /> ); @@ -81,7 +82,8 @@ function ContextField({ type="text" value={value as string} onChange={handleChange} - className="p-3 w-50 grey-border short-text" + className="p-3 grey-border short-text" + id={`context.${contextKey}`} /> ); @@ -89,10 +91,16 @@ function ContextField({ return ( + className="short-text d-flex align-items-center" + > + + ); case "Date": @@ -103,6 +111,7 @@ function ContextField({ value={value as string} onChange={handleChange} className="p-3 w-50 grey-border short-text" + id={`context.${contextKey}`} /> ); @@ -112,8 +121,9 @@ function ContextField({ as="textarea" value={value as string} onChange={handleChange} - className="p-3 w-50 grey-border long-text" + className="p-3 grey-border long-text font-monospace" placeholder="null" + id={`context.${contextKey}`} /> ); @@ -338,9 +348,14 @@ const JEXLDebuggerPage: FC = () => { Reset Context {Object.entries(originalContext).map(([key]) => ( - - - {key} + + + + {key} + Date: Tue, 7 Jul 2026 13:36:06 -0400 Subject: [PATCH 7/9] Support undefined values in the targeting context and evaluator output Previously, an undefined value (i.e., the key is present in the context with an undefined value, or in code) would not render any form control. Now values will render as an object field, i.e., as a textbox that will attempt to parse as JSON. Parsing logic has been updated to special case the string `"undefined"` to parse to `undefined` and to stringify `undefined` to `"undefined"`. When the evaluator returns `undefined`, we no display an empty output section, but instead display `"undefined"`. Fixes #231 --- src/ui/components/JEXLDebuggerPage.tsx | 16 +++++++++++++--- src/ui/jexlParser.ts | 4 +++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/ui/components/JEXLDebuggerPage.tsx b/src/ui/components/JEXLDebuggerPage.tsx index 627c065..e882647 100644 --- a/src/ui/components/JEXLDebuggerPage.tsx +++ b/src/ui/components/JEXLDebuggerPage.tsx @@ -14,7 +14,7 @@ import { useToastsContext } from "../hooks/useToasts"; import { evaluateJexl } from "../jexlParser"; import { debounce } from "../utils/functional"; -type ContextValue = object | string | boolean | number | Date; +type ContextValue = object | string | boolean | number | Date | undefined; type FieldType = "object" | "string" | "boolean" | "number" | "Date"; type FormDataValue = string | boolean; @@ -44,6 +44,8 @@ const getFieldType = (value: ContextValue): FieldType => { } else { return "object"; } + case "undefined": + return "object"; } }; @@ -128,6 +130,7 @@ function ContextField({ ); default: + console.error(`Unexpected fieldType: ${fieldType} for ${contextKey}`); return null; } } @@ -183,7 +186,11 @@ const JEXLDebuggerPage: FC = () => { formValue = (value as Date).toISOString().slice(0, 19); break; case "object": - formValue = JSON.stringify(value, null, 2); + if (typeof value === "undefined") { + formValue = "undefined"; + } else { + formValue = JSON.stringify(value, null, 2); + } break; } return [key, formValue]; @@ -246,7 +253,10 @@ const JEXLDebuggerPage: FC = () => { } } else { try { - const parsedValue = JSON.parse(value) as ContextValue; + const parsedValue = + value.trim() === "undefined" + ? undefined + : (JSON.parse(value) as ContextValue); setModifiedContext((prevContext) => ({ ...prevContext, [key]: parsedValue, diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index bd2c706..eb24d09 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -47,7 +47,9 @@ export async function evaluateJexl( await traverseAst(ast, context, falseParts); const finalResultStr = - typeof finalResult === "string" + typeof finalResult === "undefined" + ? "undefined" + : typeof finalResult === "string" ? quoteString(finalResult) : JSON.stringify(finalResult, null, 2); From b556c399224407bf5b718e3ce9b78306513c762c Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Wed, 24 Jun 2026 14:59:46 -0400 Subject: [PATCH 8/9] Identify which context attributes are not reported in telemetry When we retieve the browser's targeting context we now also retieve the list of attributes are reported in Nimbus telemetry. We use this information to to highlight which context attributes are not reported in telemetry by bading each attribute with an exclamation mark. Hovering over the badge will show a tooltip informting the user that the context attribute is not available. Some renaming has taken place inside the JEXLDebuggerPage in order to clarify what exactly each piece of state contains: * `originalContext` has been renamed to `clientContext`, which mirrors the `ClientContext` type returned by `browser.experiments.nimbus.getClientContext()`; and * `modifiedContext` has been renamed to `contextOverrides` to make it clear that it only contains individually overidden keys and not the entire context. Fixes #221 --- src/apis/nimbus.js | 19 ++- src/types/global.d.ts | 7 +- src/ui/components/JEXLDebuggerPage.tsx | 202 ++++++++++++++----------- src/ui/jexlParser.ts | 8 +- 4 files changed, 141 insertions(+), 95 deletions(-) diff --git a/src/apis/nimbus.js b/src/apis/nimbus.js index c4f83d4..d53be7e 100644 --- a/src/apis/nimbus.js +++ b/src/apis/nimbus.js @@ -28,6 +28,18 @@ ChromeUtils.defineLazyGetter(this, "_ExperimentAPI", () => { } }); +ChromeUtils.defineLazyGetter(this, "TargetingContextRecorder", () => { + try { + return ChromeUtils.importESModule( + "moz-src:///toolkit/components/nimbus/lib/TargetingContextRecorder.sys.mjs", + ); + } catch { + return ChromeUtils.importESModule( + "resource://nimbus/lib/TargetingContextRecorder.sys.mjs", + ); + } +}); + ChromeUtils.defineLazyGetter( this, "ExperimentAPI", @@ -296,7 +308,7 @@ var nimbus = class extends ExtensionAPI { ExperimentManager.createTargetingContext(), ); - const targetingParameters = { + const values = { ...environment, ...localContext, // This code is based on the implementation from: @@ -309,7 +321,10 @@ var nimbus = class extends ExtensionAPI { os: ClientEnvironmentBase.os, }; - return targetingParameters; + return { + attrs: Object.keys(TargetingContextRecorder.ATTRIBUTE_TRANSFORMS), + values, + }; }, async updateRecipes() { diff --git a/src/types/global.d.ts b/src/types/global.d.ts index 3f64529..69fc078 100644 --- a/src/types/global.d.ts +++ b/src/types/global.d.ts @@ -136,6 +136,11 @@ type CurrentCollection = { prefValue: string; }; +type ClientContextValue = object | string | boolean | number | Date | undefined; +type ClientContext = { + attrs: string[]; + values: Record; +}; declare namespace browser.experiments.nimbus { function enrollInExperiment( jsonData: object, @@ -157,7 +162,7 @@ declare namespace browser.experiments.nimbus { function evaluateJEXL(expression: string, context: object): Promise; - function getClientContext(): Promise; + function getClientContext(): Promise; function injectInactiveEnrollment( recipe: object, diff --git a/src/ui/components/JEXLDebuggerPage.tsx b/src/ui/components/JEXLDebuggerPage.tsx index e882647..31311a9 100644 --- a/src/ui/components/JEXLDebuggerPage.tsx +++ b/src/ui/components/JEXLDebuggerPage.tsx @@ -7,14 +7,23 @@ import { useMemo, useRef, } from "react"; -import { Container, Row, Col, Form, Button } from "react-bootstrap"; +import { + Badge, + Container, + Row, + Col, + Form, + Button, + Tooltip, + OverlayTrigger, + TooltipProps, +} from "react-bootstrap"; import { useLocation } from "react-router-dom"; import { useToastsContext } from "../hooks/useToasts"; import { evaluateJexl } from "../jexlParser"; import { debounce } from "../utils/functional"; -type ContextValue = object | string | boolean | number | Date | undefined; type FieldType = "object" | "string" | "boolean" | "number" | "Date"; type FormDataValue = string | boolean; @@ -30,7 +39,7 @@ type ContextFieldProps = { value: TValue; }; -const getFieldType = (value: ContextValue): FieldType => { +const getFieldType = (value: ClientContextValue): FieldType => { switch (typeof value) { case "string": return "string"; @@ -141,75 +150,66 @@ type JEXLDebuggerPageState = { const JEXLDebuggerPage: FC = () => { const mounted = useRef(false); - const [originalContext, setOriginalContext] = useState< - Record - >({}); - const [modifiedContext, setModifiedContext] = useState< - Record - >({}); - const [formData, setFormData] = useState>({}); const { state: locationState } = useLocation() as { state: JEXLDebuggerPageState | null; }; + const { addToast } = useToastsContext(); + const [clientContext, setClientContext] = useState(); + const [contextOverrides, setContextOverrides] = useState< + Record + >({}); + const [formData, setFormData] = useState>({}); const [jexlExpression, setJexlExpression] = useState( locationState?.jexlExpression ?? "", ); const [output, setOutput] = useState(""); - const { addToast } = useToastsContext(); const fetchClientContext = useCallback(async () => { - try { - const context = - (await browser.experiments.nimbus.getClientContext()) as Record< - string, - ContextValue - >; - setOriginalContext(context); - setModifiedContext({}); - setFormData( - Object.fromEntries( - Object.entries(context).map(([key, value]) => { - let formValue: FormDataValue; - const fieldType = getFieldType(value); + await browser.experiments.nimbus.getClientContext().then( + (context) => { + setClientContext(context); + setContextOverrides({}); + setFormData( + Object.fromEntries( + Object.entries(context.values).map(([key, value]) => { + let formValue: FormDataValue; + const fieldType = getFieldType(value); - switch (fieldType) { - case "string": - formValue = value as string; - break; - case "boolean": - formValue = value as boolean; - break; - case "number": - formValue = (value as number).toString(); - break; - case "Date": - formValue = (value as Date).toISOString().slice(0, 19); - break; - case "object": - if (typeof value === "undefined") { - formValue = "undefined"; - } else { - formValue = JSON.stringify(value, null, 2); - } - break; - } - return [key, formValue]; - }), - ), - ); - } catch (error) { - addToast({ - message: `Error fetching client context: ${(error as Error).message ?? String(error)}`, - variant: "danger", - }); - } + switch (fieldType) { + case "string": + formValue = value as string; + break; + case "boolean": + formValue = value as boolean; + break; + case "number": + formValue = (value as number).toString(); + break; + case "Date": + formValue = (value as Date).toISOString().slice(0, 19); + break; + case "object": + if (typeof value === "undefined") { + formValue = "undefined"; + } else { + formValue = JSON.stringify(value, null, 2); + } + break; + } + return [key, formValue]; + }), + ), + ); + }, + (error) => + addToast({ + message: `Error fetching client context: ${(error as Error).message ?? String(error)}`, + variant: "danger", + }), + ); }, [addToast]); useEffect(() => { - // This will raise a false positive about calling setState in effect, even - // though the setState happens *after* an await statement. - // See-also: https://github.com/facebook/react/issues/34905 - // eslint-disable-next-line react-hooks/set-state-in-effect void fetchClientContext(); }, [fetchClientContext]); @@ -222,8 +222,8 @@ const JEXLDebuggerPage: FC = () => { const evaluateExpression = useCallback(() => { evaluateJexl(jexlExpression, { - ...originalContext, - ...modifiedContext, + ...clientContext?.values, + ...contextOverrides, }).then( (result) => setOutput(result), (error) => { @@ -234,14 +234,14 @@ const JEXLDebuggerPage: FC = () => { }); }, ); - }, [jexlExpression, modifiedContext, originalContext, addToast]); + }, [jexlExpression, contextOverrides, clientContext, addToast]); const parseAndSetContext = useMemo(() => { return debounce((key: string, value: string, isNumber: boolean) => { if (isNumber) { const numVal = parseInt(value); if (!isNaN(numVal)) { - setModifiedContext((prevContext) => ({ + setContextOverrides((prevContext) => ({ ...prevContext, [key]: numVal, })); @@ -256,8 +256,8 @@ const JEXLDebuggerPage: FC = () => { const parsedValue = value.trim() === "undefined" ? undefined - : (JSON.parse(value) as ContextValue); - setModifiedContext((prevContext) => ({ + : (JSON.parse(value) as ClientContextValue); + setContextOverrides((prevContext) => ({ ...prevContext, [key]: parsedValue, })); @@ -283,21 +283,21 @@ const JEXLDebuggerPage: FC = () => { break; case "boolean": - setModifiedContext((prevContext) => ({ + setContextOverrides((prevContext) => ({ ...prevContext, [key]: value, })); break; case "Date": - setModifiedContext((prevContext) => ({ + setContextOverrides((prevContext) => ({ ...prevContext, [key]: new Date(value as string), })); break; case "string": - setModifiedContext((prevContext) => ({ + setContextOverrides((prevContext) => ({ ...prevContext, [key]: value, })); @@ -357,30 +357,56 @@ const JEXLDebuggerPage: FC = () => { > Reset Context - {Object.entries(originalContext).map(([key]) => ( - - - - {key} - - - - - - - ))} + {clientContext && + Object.entries(clientContext.values).map(([key]) => ( + + + + {key} + + {clientContext.attrs.includes(key) || ( + + + + ! + + + + )} + + + + + + ))} ); }; +function NotInMetricsWarning(props: TooltipProps): JSX.Element { + return ( + + This attribute is not reported in Nimbus targeting context telemetry and + is not indexed by Experimenter. + + ); +} + export default JEXLDebuggerPage; diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index eb24d09..4d9fcc3 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -48,10 +48,10 @@ export async function evaluateJexl( const finalResultStr = typeof finalResult === "undefined" - ? "undefined" - : typeof finalResult === "string" - ? quoteString(finalResult) - : JSON.stringify(finalResult, null, 2); + ? "undefined" + : typeof finalResult === "string" + ? quoteString(finalResult) + : JSON.stringify(finalResult, null, 2); const falsePartsStr = finalResult === false ? `\n\nFalse Parts:\n${falseParts.join("\n")}` : ""; From e20440f9abb4685727bd093de60d45bb15d68e35 Mon Sep 17 00:00:00 2001 From: Beth Rennie Date: Fri, 26 Jun 2026 11:25:56 -0400 Subject: [PATCH 9/9] Report unrecorded attributes and prefs from evaluated expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `debugJexl` (née `evaluateJexl`) now returns an object containing debugging information about the expression instead of a stringified representation of the display payload. `traverseAst` has been renamed to `collectFalseExprs` because we are now doing multiple additional traversals with `collectAttrsAndPrefs`. Fixes #230 --- src/apis/nimbus.js | 1 + src/types/global.d.ts | 1 + src/ui/components/JEXLDebuggerPage.tsx | 105 +++++++++++++-- src/ui/jexlParser.ts | 176 +++++++++++++++++++------ 4 files changed, 233 insertions(+), 50 deletions(-) diff --git a/src/apis/nimbus.js b/src/apis/nimbus.js index d53be7e..d775a0a 100644 --- a/src/apis/nimbus.js +++ b/src/apis/nimbus.js @@ -323,6 +323,7 @@ var nimbus = class extends ExtensionAPI { return { attrs: Object.keys(TargetingContextRecorder.ATTRIBUTE_TRANSFORMS), + prefs: Object.keys(TargetingContextRecorder.PREFS), values, }; }, diff --git a/src/types/global.d.ts b/src/types/global.d.ts index 69fc078..0a6b8be 100644 --- a/src/types/global.d.ts +++ b/src/types/global.d.ts @@ -139,6 +139,7 @@ type CurrentCollection = { type ClientContextValue = object | string | boolean | number | Date | undefined; type ClientContext = { attrs: string[]; + prefs: string[]; values: Record; }; declare namespace browser.experiments.nimbus { diff --git a/src/ui/components/JEXLDebuggerPage.tsx b/src/ui/components/JEXLDebuggerPage.tsx index 31311a9..4b68a47 100644 --- a/src/ui/components/JEXLDebuggerPage.tsx +++ b/src/ui/components/JEXLDebuggerPage.tsx @@ -17,11 +17,12 @@ import { Tooltip, OverlayTrigger, TooltipProps, + Alert, } from "react-bootstrap"; import { useLocation } from "react-router-dom"; import { useToastsContext } from "../hooks/useToasts"; -import { evaluateJexl } from "../jexlParser"; +import { debugJexl, DebugJexlResult } from "../jexlParser"; import { debounce } from "../utils/functional"; type FieldType = "object" | "string" | "boolean" | "number" | "Date"; @@ -148,6 +149,10 @@ type JEXLDebuggerPageState = { jexlExpression?: string; }; +type EvalState = + | ({ ok: true } & DebugJexlResult) + | { ok: false; error: string }; + const JEXLDebuggerPage: FC = () => { const mounted = useRef(false); const { state: locationState } = useLocation() as { @@ -162,7 +167,7 @@ const JEXLDebuggerPage: FC = () => { const [jexlExpression, setJexlExpression] = useState( locationState?.jexlExpression ?? "", ); - const [output, setOutput] = useState(""); + const [evalResult, setEvalResult] = useState(null); const fetchClientContext = useCallback(async () => { await browser.experiments.nimbus.getClientContext().then( @@ -221,20 +226,19 @@ const JEXLDebuggerPage: FC = () => { ); const evaluateExpression = useCallback(() => { - evaluateJexl(jexlExpression, { + debugJexl(jexlExpression, { ...clientContext?.values, ...contextOverrides, }).then( - (result) => setOutput(result), + (result) => setEvalResult({ ...result, ok: true }), (error) => { - setOutput("Evaluation error"); - addToast({ - message: `Error evaluating expression: ${(error as Error).message ?? String(error)}`, - variant: "danger", + setEvalResult({ + error: error instanceof Error ? error.message : String(error), + ok: false, }); }, ); - }, [jexlExpression, contextOverrides, clientContext, addToast]); + }, [jexlExpression, contextOverrides, clientContext]); const parseAndSetContext = useMemo(() => { return debounce((key: string, value: string, isNumber: boolean) => { @@ -319,6 +323,30 @@ const JEXLDebuggerPage: FC = () => { mounted.current = true; }, [locationState?.jexlExpression, evaluateExpression]); + const { unreportedAttrs, unreportedPrefs } = useMemo(() => { + const unreportedAttrs: string[] = []; + const unreportedPrefs: string[] = []; + + if (evalResult?.ok && clientContext) { + unreportedAttrs.push( + ...Array.from(evalResult.attrs.values()).filter( + (attr) => !clientContext.attrs.includes(attr), + ), + ); + + unreportedPrefs.push( + ...Array.from(evalResult.prefs.values()).filter( + (pref) => !clientContext.prefs.includes(pref), + ), + ); + } + + return { + unreportedAttrs: unreportedAttrs.length ? unreportedAttrs : null, + unreportedPrefs: unreportedPrefs.length ? unreportedPrefs : null, + }; + }, [evalResult, clientContext]); + return ( @@ -343,8 +371,65 @@ const JEXLDebuggerPage: FC = () => {
+ {unreportedAttrs && ( + + + Targeting Expression includes Unreported Attributes + +

+ This targeting expression includes attributes that are not + reported in Nimbus targeting context telemetry and are not + indexed by Experimenter: +

+
    + {unreportedAttrs.map((attr) => ( +
  • + {attr} +
    • + ))} +
+ + )} + {unreportedPrefs && ( + + + Targeting Expression includes Unreported Prefs + +

+ This targeting expression includes prefs that are not reported + in Nimbus targeting context telemetry and are not indexed by + Experimenter: +

+
    + {unreportedPrefs.map((pref) => ( +
  • + {pref} +
    • + ))} +
+ + )}

Output

- 
{output}
+ {evalResult ? ( + evalResult.ok ? ( + 
{evalResult.value}
+ ) : ( +

Could not evaluate JEXL: {evalResult.error}

+ ) + ) : null} + {evalResult?.ok && evalResult.falseExprs.length > 0 ? ( + <> +

False sub-expressions

+

The following sub-expressions evaluated to false:

+
    + {evalResult.falseExprs.map((value, i) => ( +
  • + {value} +
    • + ))} +
+ + ) : null}
diff --git a/src/ui/jexlParser.ts b/src/ui/jexlParser.ts index 4d9fcc3..1d6fe6d 100644 --- a/src/ui/jexlParser.ts +++ b/src/ui/jexlParser.ts @@ -1,6 +1,6 @@ import { elements as baseGrammar } from "mozjexl/lib/grammar"; import Lexer from "mozjexl/lib/Lexer"; -import Parser, { ASTNode } from "mozjexl/lib/parser/Parser"; +import Parser, { ASTNode, Identifier } from "mozjexl/lib/parser/Parser"; const grammar = { ...baseGrammar, @@ -19,6 +19,16 @@ const grammar = { }, }; +type DebugContext = { + falseExprs: string[]; + attrs: Set; + prefs: Set; +}; + +export type DebugJexlResult = DebugContext & { + value: string; +}; + /** * Evaluates a JEXL expression within a given context. * @@ -27,44 +37,50 @@ const grammar = { * * @returns The result of the evaluation as a string. */ -export async function evaluateJexl( +export async function debugJexl( expression: string, context: object = {}, -): Promise { +): Promise { if (!expression.trim()) { throw new Error("Empty expression"); } - const finalResult = await evaluateExpression(expression, context); + const value = await evaluateExpression(expression, context); const lexer = new Lexer(grammar); const parser = new Parser(grammar); parser.addTokens(lexer.tokenize(expression)); - const ast = parser.complete(); - const falseParts: string[] = []; - await traverseAst(ast, context, falseParts); + const debugCtx = { + falseExprs: [], + attrs: new Set(), + prefs: new Set(), + } as DebugContext; - const finalResultStr = - typeof finalResult === "undefined" - ? "undefined" - : typeof finalResult === "string" - ? quoteString(finalResult) - : JSON.stringify(finalResult, null, 2); + const ast = parser.complete(); + + await collectFalseExprs(ast, context, debugCtx.falseExprs); + collectAttrsAndPrefs(ast, debugCtx); - const falsePartsStr = - finalResult === false ? `\n\nFalse Parts:\n${falseParts.join("\n")}` : ""; - return `${finalResultStr}${falsePartsStr}`; + return { + ...debugCtx, + value: + typeof value === "undefined" + ? "undefined" + : typeof value === "string" + ? quoteString(value) + : JSON.stringify(value, null, 2), + }; } /** - * Evaluates a JEXL expression using the browser's Nimbus API. + * Debug a JEXL expression using the browser's Nimbus API. * * @param expression - The JEXL expression to evaluate. * @param context - The context in which to evaluate the expression. * - * @returns The result of the evaluation. + * @returns Debugging information about the expression. */ async function evaluateExpression( expression: string, @@ -86,19 +102,15 @@ async function evaluateExpression( * * @param ast The AST node to traverse. * @param context The context in which to evaluate the expressions. - * @param falseParts An array to collect expressions that evaluate to false. + * @param falseExprs An array to collect expressions that evaluate to false. * * @returns The result of the traversal. */ -async function traverseAst( +async function collectFalseExprs( ast: ASTNode, context: object, - falseParts: string[], + falseExprs: string[], ): Promise { - if (!ast) { - return true; - } - const subExpr = getExpression(ast); const result = await evaluateExpression(subExpr, context); @@ -113,13 +125,13 @@ async function traverseAst( ); if (result === false && leftResult !== false && rightResult !== false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); } else { if (ast.left) { - await traverseAst(ast.left, context, falseParts); + await collectFalseExprs(ast.left, context, falseExprs); } if (ast.right) { - await traverseAst(ast.right, context, falseParts); + await collectFalseExprs(ast.right, context, falseExprs); } } } else if (ast.type === "UnaryExpression") { @@ -128,49 +140,49 @@ async function traverseAst( context, ); if (result === false && rightResult !== false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); if (ast.right) { - await traverseAst(ast.right, context, falseParts); + await collectFalseExprs(ast.right, context, falseExprs); } } else { if (ast.right) { - await traverseAst(ast.right, context, falseParts); + await collectFalseExprs(ast.right, context, falseExprs); } } } else if (ast.type === "Transform") { if (result === false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); } else { if (ast.subject) { - await traverseAst(ast.subject, context, falseParts); + await collectFalseExprs(ast.subject, context, falseExprs); } if (ast.args) { for (const arg of ast.args) { - await traverseAst(arg, context, falseParts); + await collectFalseExprs(arg, context, falseExprs); } } } } else if (ast.type === "FilterExpression") { if (result === false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); } else { if (ast.subject) { - await traverseAst(ast.subject, context, falseParts); + await collectFalseExprs(ast.subject, context, falseExprs); } if (ast.expr) { - await traverseAst(ast.expr, context, falseParts); + await collectFalseExprs(ast.expr, context, falseExprs); } } } else if (ast.type === "Literal" || ast.type === "Identifier") { if (result === false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); } } else if (ast.type === "ObjectLiteral") { if (result === false) { - falseParts.push(subExpr); + falseExprs.push(subExpr); } else { for (const key in ast.value) { - await traverseAst(ast.value[key], context, falseParts); + await collectFalseExprs(ast.value[key], context, falseExprs); } } } @@ -178,6 +190,90 @@ async function traverseAst( return result; } +function collectAttrsAndPrefs(ast: ASTNode, debugCtx: DebugContext) { + switch (ast.type) { + case "BinaryExpression": + case "LogicalExpression": + collectAttrsAndPrefs(ast.left, debugCtx); + collectAttrsAndPrefs(ast.right, debugCtx); + break; + + case "UnaryExpression": + collectAttrsAndPrefs(ast.right, debugCtx); + break; + + case "Transform": + if ( + ast.name === "preferenceValue" && + ast.subject.type === "Literal" && + typeof ast.subject.value === "string" + ) { + debugCtx.prefs.add(ast.subject.value); + } else { + collectAttrsAndPrefs(ast.subject, debugCtx); + } + break; + + case "FilterExpression": + collectAttrsAndPrefs(ast.expr, debugCtx); + collectAttrsAndPrefs(ast.subject, debugCtx); + break; + + case "Identifier": + { + const attr = getRootAttribute(ast); + if (attr) { + debugCtx.attrs.add(attr); + } + } + break; + + case "ObjectLiteral": + for (const valueNode of Object.values(ast.value)) { + collectAttrsAndPrefs(valueNode, debugCtx); + } + break; + + case "ArrayLiteral": + for (const itemNode of ast.value) { + collectAttrsAndPrefs(itemNode, debugCtx); + } + break; + + case "Literal": + break; + + default: + // TypeScript correctly deduces `ast: never`, so we have to re-cast it to an + // ASTNode to print this error. + throw new TypeError(`Unexpected AST node type ${(ast as ASTNode).type}`); + } +} + +/** + * Return the root attribute of a given indentifier, if it exists. + * + * For example, for the identifier "foo.bar.baz", this will return "foo". + * + * However, for an identifier on, e.g., an object or array literal, this will + * return null. + * + * @param ast The AST node for the identifier. + * + * @returns The root identifier + * */ +function getRootAttribute(ast: Identifier): string | null { + if (!ast.from) { + return ast.value; + } + + if (ast.from.type === "Identifier") { + return getRootAttribute(ast.from); + } + + return null; +} + /** * Converts an AST node to its corresponding JEXL expression string. *