firefox-devtools / firefox-devtools/profiler
Make formatting types generic and extensible
- Dominant language
- TypeScript
- Stars
- 1.5k
- Forks
- 491
- Avg merge
- 3d 46m
- Merged PRs (30d)
- 27
Description
Formatting of values, used for markers and in the future for additional profile information (see #4222), is currently not self contained. But for every formatting rule, we essentially have two functions:
* `formatAsString(value: any): string` and
* `formatAsElement(value: any): React.Element | string` which also supports returning HTML, use `formatAsString` if `undefined`
A format type can currently be generalized as either a string or an object with a `type` property that gives us the type name.
A format can therefore be specified as something like:
```js
{
type: string,
isObjectType: boolean,
formatAsString: (value: any) => string,
formatAsElement(value: any): React.Element | string
}
```
We would for the type `url` specify for example:
```jsx
{
type: "url",
isObjectType: false,
formatAsString: (value) => String(value),
formatAsElement?: (value) => {value}
}
```
We then only need a small registry with a `registerFormat({...})` function which registers the format and a `getFormat(type: string)` function which returns a format object or `null` if the format is not registered. The `registerFormat` functions emits a warning if a format is already registered, but overrides it anyway.
This in itself will improve the readability of the formatting related code and makes adding new formats easier.
But more importantly, it allows us to implement custom marker formats at compile/bundle time:
The motivation behind this is that I would to add more formatting types to my custom distribution of Firefox Profiler.
This distribution is focused on a specific set of imported profiles. But adding the additional types currently consists
of manually tracking all changes to `marker-schema.js` with potentially costly merging everytime this file is changed.
There a two technical challenges which I cover in the following: how to implement it and alt texts.
Implementing it is pretty easy: We can just have a folder `src/custom` with an empty `index.js` and we include this folder using `import 'custom'` in the `src/index.js` file after all other imports. The custom formatter types can then registered using the `registerFormat({...})` function which is called somewhere in `src/custom`.
Custom formatting types are by definition not supported by the upstream project (but might eventually be merged), this causes a problem when viewing profiles with these types in the [profiler.firefox.com](https://profiler.firefox.com) instance: We have to offer a way to support graceful degradation. This leads me to my proposal of the format of custom format types which include the name (in `type` as before), a possible alternative type which can be used instead of this type and a text to show when the type is not supported (in `altText`):
```jsx
{
type: string,
altType?: FormatType
altText?: string
}
```
If neither `altType` nor `altText` are given, then fields with this type should be omitted. This is usually the case for additional information which is not that important. The `altType` always takes precendence of the `altText` (but having an `altType` and an `altText` should trigger a warning).
An example could be a date type: Currently the times are formatted relative to the start time of the profile. We could implement this type as
```js
{
type: "date",
altType: "time"
}
```
and implement it in `src/custom/index.js` as follows:
```js
registerFormat({
type: "date",
isObjectType: false,
formatAsString: (value) => /* date to string */
})
```
This proposal would make it possible to implement formatting types in a more generic and in extensible way, making them a corner stone for value formatting in Firefox Profiler.
_This will be the first issue in a series to explore the possibilities for making Firefox Profiler more extensible._
┆Issue is synchronized with this [Jira Task](https://mozilla-hub.atlassian.net/browse/FP-581)
Contributor guide
Assessment
This issue has not been assessed yet.