refactor(devtools): rename timing API to performance track#69733
refactor(devtools): rename timing API to performance track#69733hawkgs wants to merge 2 commits into
Conversation
hawkgs
commented
Jul 10, 2026
- Rename Timing API to Performance Track to better reflect the purpose of the actual feature
- Migrate the settings data object to match the new name and drop some redundant keys
Rename Timing API to Performance Track to better reflect the purpose of the actual feature; Migrate the settings data object to match the new name and drop some redundant keys.
dgp1130
left a comment
There was a problem hiding this comment.
I would also be fine with just updating the text and keep using the old name in the setting store and message bus, but they're reasonable refactors and it's good to clean up some of the extra settings data anyways, so LGTM.
I would recommend doing some extra manual testing on the settings store migration side if you haven't already, just because we don't have much experience making this kind of change and it seems like an easy thing to mess up. I don't see anything obviously wrong here and appreciate the tests for it, just want to be extra safe here.
|
|
||
| // V1 -> V2 migration | ||
| if (dataVer === 1) { | ||
| const newData = dataCopy; |
There was a problem hiding this comment.
Nit: newData and dataCopy sound like distinct objects with distinct values, but these are all direct mutations of the same object. I recommend either mutating dataCopy directly or doing an immutable update like:
const newData = {
...dataCopy,
['performance_track@profiling']: dataCopy['timing_api_enabled@general'],
['activeTab@general']: undefined,
// ...
};I guess that might write undefined rather than deleting the property, so maybe the mutation option with clear names is better?
There was a problem hiding this comment.
Oh, it's a left over since I was playing with some types here; hence, the new newData. It should've been dataCopy as originally intended.
| // 2. We have a complete historical record of legacy data objects. | ||
| type SettingsData = SettingsDataV1 | SettingsDataV2; | ||
|
|
||
| interface SettingsDataV2 { |
There was a problem hiding this comment.
Consider: Is there a path to making this a type which is inferred from the Settings class to reduce the chance of divergence from the real shape of this data?
There was a problem hiding this comment.
I was thinking about the same, but I couldn't come up with a way to do it.
The only thing that could make this probably feasible is to change the design of Settings – instead of using separate key and category properties when defining a settings, I can use just a key: 'foo@bar' instead. This will mirror 1:1 how the data is kept in the storage, respectively, I'll be able to use the version interfaces.
I assume that the net positive effects of this change will outweigh the benefits of the current separate-key-category-props API.
Edit:
Woah, after consulting with Gemini, it seems that TS type system can actually achieve this (TIL):
type Foo = 'bar@baz';
// Utility type to infer and extract the parts
type Split<T> = T extends `${infer Left}@${infer Right}`
? { prefix: Left; suffix: Right }
: never;
type Result = Split<Foo>;
// Result evaluates to: { prefix: "bar"; suffix: "baz" }