Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/profile-logic/marker-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ export function getSchemaFromMarker(
return schemaName ? (markerSchemaByName[schemaName] ?? null) : null;
}

// Matches ternary expressions inside marker labels, ie {marker.data.field ? 'truthy' : 'falsy'}
const TERNARY_RE = /^\s*([\w.]+)\s*\?\s*'([^']*)'\s*:\s*'([^']*)'\s*$/;

/**
* Marker schema can create a dynamic tooltip label. For instance a schema with
* a `tooltipLabel` field of "Event at {marker.data.url}" would create a label based
Expand Down Expand Up @@ -142,8 +145,9 @@ export function parseLabel(
console.error(oneLine`
Error processing the label "${label}" because of the ${part}.
Currently the labels in the marker schema take the form of
"marker.data.keyName" or "marker.startTime". No other type
of access is currently supported.
"marker.data.keyName", "marker.startTime", or a ternary like
"marker.data.keyName ? 'yes' : 'no'". No other type of access
is currently supported.
`);
return () => '';
}
Expand All @@ -161,6 +165,29 @@ export function parseLabel(
// Given: "Marker information: {marker.name} – {marker.data.info}"
// Handle: ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^

// Check for a ternary expression:
// Given: "{marker.data.canceled ? '❌' : ''} {marker.data.delay}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we went with the single quotes, which I think makes sense considering the C++ strings. It might also make sense to update the marker documentation that we have in Firefox to mention this new feature: https://firefox-source-docs.mozilla.org/tools/profiler/markers-guide.html#:~:text=The%20arguments%20is%20a%20string%20that%20may%20refer%20to%20marker%20data%20within%20braces

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do that 👍🏻 Would also be a good opportunity for me to learn that process 😀

// Handle: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//
// Only marker.data.* fields are supported as conditions. The true/false
// branches must be single-quoted string literals.
const ternaryMatch = part.match(TERNARY_RE);
if (ternaryMatch) {
const [, condRef, truthyStr, falsyStr] = ternaryMatch;
const condKeys = condRef.split('.');
if (
condKeys.length === 3 &&
condKeys[0] === 'marker' &&
condKeys[1] === 'data' &&
condKeys[2]
) {
const condPayloadKey = condKeys[2];
return (marker) =>
(marker.data as any)?.[condPayloadKey] ? truthyStr : falsyStr;
}
return parseError(label, part);
}

const keys = part.trim().split('.');

if (keys.length !== 2 && keys.length !== 3) {
Expand Down
8 changes: 4 additions & 4 deletions src/test/unit/__snapshots__/marker-schema.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -259,31 +259,31 @@ Array [
exports[`marker schema labels parseErrors errors if looking up into a part of the marker that does not exist 1`] = `
Array [
Array [
"Error processing the label \\"Parse error: \\"{marker.nothing}\\"\\" because of the marker.nothing. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\" or \\"marker.startTime\\". No other type of access is currently supported.",
"Error processing the label \\"Parse error: \\"{marker.nothing}\\"\\" because of the marker.nothing. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\", \\"marker.startTime\\", or a ternary like \\"marker.data.keyName ? 'yes' : 'no'\\". No other type of access is currently supported.",
],
]
`;

exports[`marker schema labels parseErrors errors if not looking up into a marker 1`] = `
Array [
Array [
"Error processing the label \\"Parse error: \\"{duration}\\"\\" because of the duration. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\" or \\"marker.startTime\\". No other type of access is currently supported.",
"Error processing the label \\"Parse error: \\"{duration}\\"\\" because of the duration. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\", \\"marker.startTime\\", or a ternary like \\"marker.data.keyName ? 'yes' : 'no'\\". No other type of access is currently supported.",
],
]
`;

exports[`marker schema labels parseErrors errors when accessing random properties 1`] = `
Array [
Array [
"Error processing the label \\"Parse error: \\"{property.value}\\"\\" because of the property.value. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\" or \\"marker.startTime\\". No other type of access is currently supported.",
"Error processing the label \\"Parse error: \\"{property.value}\\"\\" because of the property.value. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\", \\"marker.startTime\\", or a ternary like \\"marker.data.keyName ? 'yes' : 'no'\\". No other type of access is currently supported.",
],
]
`;

exports[`marker schema labels parseErrors errors when accessing twice into a payload 1`] = `
Array [
Array [
"Error processing the label \\"Parse error: \\"{marker.data.duration.extra}\\"\\" because of the marker.data.duration.extra. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\" or \\"marker.startTime\\". No other type of access is currently supported.",
"Error processing the label \\"Parse error: \\"{marker.data.duration.extra}\\"\\" because of the marker.data.duration.extra. Currently the labels in the marker schema take the form of \\"marker.data.keyName\\", \\"marker.startTime\\", or a ternary like \\"marker.data.keyName ? 'yes' : 'no'\\". No other type of access is currently supported.",
],
]
`;
75 changes: 75 additions & 0 deletions src/test/unit/marker-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,81 @@ describe('marker schema labels', function () {
expect(console.error).toHaveBeenCalledTimes(0);
});

describe('ternary expressions', function () {
it('returns the truthy string when the field is truthy', function () {
expect(
applyLabel({
label: "{marker.data.canceled ? '❌' : ''}",
schemaFields: [
{ key: 'canceled', label: 'Canceled', format: 'string' },
],
payload: { canceled: true },
})
).toEqual('❌');
expect(console.error).toHaveBeenCalledTimes(0);
});

it('returns the falsy string when the field is falsy', function () {
expect(
applyLabel({
label: "{marker.data.canceled ? '❌' : ''}",
schemaFields: [
{ key: 'canceled', label: 'Canceled', format: 'string' },
],
payload: { canceled: false },
})
).toEqual('');
expect(console.error).toHaveBeenCalledTimes(0);
});

it('returns the falsy string when the field is absent from the payload', function () {
expect(
applyLabel({
label: "{marker.data.canceled ? '❌' : ''}",
schemaFields: [],
payload: {},
})
).toEqual('');
expect(console.error).toHaveBeenCalledTimes(0);
});

it('returns the falsy string when there is no payload', function () {
expect(
applyLabel({
label: "{marker.data.canceled ? 'yes' : 'no'}",
schemaFields: [],
payload: null,
})
).toEqual('no');
expect(console.error).toHaveBeenCalledTimes(0);
});

it('can mix a ternary with regular field lookups', function () {
expect(
applyLabel({
label: "{marker.data.canceled ? '❌' : ''} {marker.data.delay}",
schemaFields: [
{ key: 'canceled', label: 'Canceled', format: 'string' },
{ key: 'delay', label: 'Delay', format: 'milliseconds' },
],
payload: { canceled: true, delay: 250 },
})
).toEqual('❌ 250ms');
expect(console.error).toHaveBeenCalledTimes(0);
});

it('errors when the condition is not a marker.data.* reference', function () {
expect(
applyLabel({
label: "{marker.name ? 'yes' : 'no'}",
schemaFields: [],
payload: {},
})
).toEqual('');
expect(console.error).toHaveBeenCalledTimes(1);
});
});

describe('parseErrors', function () {
function testParseError(label: string) {
expect(
Expand Down