feat: add mutation logging to DevTools profiler #4544

nolanlawson · 2024-09-10T16:01:52Z

Details

Adds mutation tracking to the existing DevTools extensions (#4535, #4541), so component authors can figure out why a component was re-rendered.

For all lwc-rehydrate events, there is a list of which components were mutated, and which properties on those components were mutated.

Does this pull request introduce a breaking change?

😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

🔬 Yes, it does include an observable change.

Only in dev mode, and only if you're looking at perf timings.

The overhead of these new APIs should also be minimal – I counted the following times when running our full Karma test suite:

getMutationProperties - 12.5ms
trackTargetForMutationLogging – 0.3ms
flushMutationLogsForVM – 0.9ms

wjhsf · 2024-09-11T13:38:01Z

packages/@lwc/engine-core/src/framework/mutation-logger.ts

+    if (isUndefined(vm)) {
+        // VM should only be undefined in vitest tests, where a reactive observer is not always associated with a VM
+        // because the unit tests just create Reactive Observers on-the-fly.
+        if (process.env.NODE_ENV === 'test-karma-lwc') {


Suggested change

if (process.env.NODE_ENV === 'test-karma-lwc') {

if (process.env.NODE_ENV !== 'test') {

If we want to assert we're not in vitest, shouldn't we check for that, rather than checking we are in Karma?

Nope, because I don't want this throwing Errors in our downstream consumer's Jest/Vitest code. :) Whereas with test-karma-lwc we know it's only ever going to throw in our own tests.

I can add a comment!

packages/@lwc/engine-core/src/framework/mutation-logger.ts

wjhsf · 2024-09-11T13:43:27Z

packages/@lwc/engine-core/src/framework/mutation-logger.ts

+                trackTargetForMutationLogging(`${toString(key)}[${i}]`, target[i]);
+            }
+        } else {
+            for (const prop of keys(target)) {


Do we care about tracking non-enumerable props?

I don't believe these are tracked by @track but I'll confirm.

Dang you're right, LWC supports mutation on non-enumerable props. Looks like we might have to dig deep here.

Interestingly, LWC doesn't seem to support Symbols as shallow reactive props, but it does work for deep reactivity. And non-enumerable props work in that case too. (Non-emuerable props don't seem to work shallowly either – they throw an error of Cannot assign to read only property when you try to write to them, even if they're writable.)

In any case this is fixed now!

wjhsf · 2024-09-11T13:45:01Z

packages/@lwc/engine-core/src/framework/mutation-logger.ts

+            throw new Error('The VM should always be defined except possibly in unit tests');
+        }
+    } else {
+        // Human-readable prop like `items[0].name` on a deep object/array


Not gonna handle {"annoying edge case": true}? 😛

Actually we don't need to handle this, because (today at least) there's no way to reference object["prop with spaces"] in a template, so it won't be reactive.

We could revisit this after CTE though so that you don't see things like object.prop with spaces in DevTools.

Ah wait, I'm wrong, you can do this with getters. I'll fix it.

ngl i assumed from the code comment that it's just a label and doesn't actually matter if it's valid JS property access...

That's true! But we may as well make the label pretty. 🤓

packages/@lwc/engine-core/src/framework/mutation-logger.ts

packages/@lwc/engine-core/src/framework/profiler.ts

wjhsf · 2024-09-11T14:05:10Z

packages/@lwc/engine-core/src/libs/mutation-tracker/index.ts

@@ -39,6 +40,9 @@ export function valueMutated(target: object, key: PropertyKey) {
        if (!isUndefined(reactiveObservers)) {
            for (let i = 0, len = reactiveObservers.length; i < len; i += 1) {
                const ro = reactiveObservers[i];
+                if (process.env.NODE_ENV !== 'production') {
+                    logMutation(ro, target, key);


ro just makes me think of a dog barking...

wjhsf · 2024-09-11T14:09:12Z

packages/@lwc/integration-karma/test/profiler/mutation-logging/x/parent/parent.html

@@ -0,0 +1,20 @@
+<template>
+    <div><strong>First name:</strong> {firstName}</div>


<div><strong>

That would've been a great parody bracelet, back in the day.

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

nolanlawson · 2024-09-11T16:11:41Z

TODO: need to add handling for experimental Signals.

nolanlawson · 2024-09-12T00:23:00Z

Tweaked the colors a bit:

nolanlawson added 4 commits September 9, 2024 14:06

feat(engine): tooltips

924792c

feat: add mutation logging to DevTools profiler

7a2bd36

test: start adding tests

a6b6c71

fix: more debuggable names for deep props

c423c9d

Base automatically changed from nolan/tooltips to master September 10, 2024 18:08

nolanlawson added 10 commits September 10, 2024 12:00

Merge remote-tracking branch 'origin/master' into nolan/mutation-logging

8e3addc

fix: error tracking just in case

a9372c5

fix: do full path for deep objects

2535573

refactor: comments

523e8e2

test: better tests

02f2189

test: fix unit tests

56a3e1f

fix: synthetic lifecycle

93c25f9

refactor: comments and tests

256c11f

refactor: comments and tests

3044f0f

refactor: refactor

98744dd

nolanlawson marked this pull request as ready for review September 10, 2024 23:22

nolanlawson requested a review from a team as a code owner September 10, 2024 23:22

wjhsf approved these changes Sep 11, 2024

View reviewed changes

nolanlawson and others added 6 commits September 11, 2024 08:57

Update packages/@lwc/engine-core/src/framework/mutation-logger.ts

ca87b40

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

Update packages/@lwc/engine-core/src/framework/profiler.ts

7989ae6

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

Update packages/@lwc/engine-core/src/framework/profiler.ts

162a968

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

Update packages/@lwc/engine-core/src/framework/mutation-logger.ts

2e5f907

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

Update packages/@lwc/engine-core/src/framework/profiler.ts

ffeb788

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

Update packages/@lwc/engine-core/src/framework/profiler.ts

c736bca

Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>

nolanlawson added 5 commits September 11, 2024 09:24

fix: import

2894807

fix: comment

6263b44

fix: code cleanup

ddef333

fix: handle bracket notation

a1138ba

feat: more elegant handling of duplicates

576b9bb

fix: dead code

1f2e120

nolanlawson mentioned this pull request Sep 11, 2024

Support experimental Signals in mutation logging for DevTools #4546

Open

nolanlawson added 8 commits September 11, 2024 10:54

fix: todo for signals

62981f6

fix: support symbols and non-enumerated props

b5cb353

fix: cleanup for symbols/wacky accessors

6e895ad

fix: simplify ternary

c7dcf3f

fix: recursive objects

561d149

fix: change color and fix symbol prop display

0117c6e

fix: typo

c300249

fix: simplify language

4e322ab

nolanlawson added 2 commits September 11, 2024 17:23

fix: tweak colors

22eceb4

refactor: make the core more DRY

36f3e6a

nolanlawson merged commit 240034a into master Sep 12, 2024
11 checks passed

nolanlawson deleted the nolan/mutation-logging branch September 12, 2024 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add mutation logging to DevTools profiler #4544

feat: add mutation logging to DevTools profiler #4544

nolanlawson commented Sep 10, 2024 •

edited

Loading

wjhsf Sep 11, 2024

nolanlawson Sep 11, 2024

wjhsf Sep 11, 2024

nolanlawson Sep 11, 2024

nolanlawson Sep 11, 2024

nolanlawson Sep 11, 2024

nolanlawson Sep 11, 2024

wjhsf Sep 11, 2024

nolanlawson Sep 11, 2024

nolanlawson Sep 11, 2024

nolanlawson Sep 11, 2024

wjhsf Sep 11, 2024

nolanlawson Sep 11, 2024

wjhsf Sep 11, 2024

wjhsf Sep 11, 2024

nolanlawson commented Sep 11, 2024

nolanlawson commented Sep 12, 2024

	if (process.env.NODE_ENV === 'test-karma-lwc') {
	if (process.env.NODE_ENV !== 'test') {

		@@ -0,0 +1,20 @@
		<template>
		<div><strong>First name:</strong> {firstName}</div>

feat: add mutation logging to DevTools profiler #4544

feat: add mutation logging to DevTools profiler #4544

Conversation

nolanlawson commented Sep 10, 2024 • edited Loading

Details

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson commented Sep 11, 2024

nolanlawson commented Sep 12, 2024

nolanlawson commented Sep 10, 2024 •

edited

Loading