Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(devtools): state diffing #1585

Open
wants to merge 6 commits into
base: v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
277 changes: 277 additions & 0 deletions packages/pinia/__tests__/devtools/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
import { describe, it, expect } from 'vitest'
import { formatStateDifferences, realTypeOf } from '../../src/devtools/utils'

describe('Devtools utils', () => {

describe('realTypeOf', () => {
it('Should correctly predict type of subject', () => {
const number = 0
const string = 'undefined'
const undefinedValue = undefined
const nullValue = null
const array: any[] = []
const date = new Date(123)
const object = {}
const regexp = /regexp/
const functionValue = () => {}

let type = realTypeOf(number)

expect(type).toEqual('number')

type = realTypeOf(string)

expect(type).toEqual('string')

type = realTypeOf(undefinedValue)

expect(type).toEqual('undefined')

type = realTypeOf(nullValue)

expect(type).toEqual('null')

type = realTypeOf(array)

expect(type).toEqual('array')

type = realTypeOf(date)

expect(type).toEqual('date')

type = realTypeOf(object)

expect(type).toEqual('object')

type = realTypeOf(regexp)

expect(type).toEqual('regexp')

type = realTypeOf(functionValue)

expect(type).toEqual('function')
})
})

describe('formatStateDifferences', () => {
it('Should find removed entries', () => {
const oldState = {
removed: 'old'
}
const newState = {}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
removed: undefined,
})
})

it('Should find difference in array', () => {
const oldState = {
changedArray1: [1, 2, 3],
unchangedArray: [1, 2, 3],
changedArray2: [1, 2, 3]
}
const newState = {
changedArray1: [1, 2, 3, 4],
unchangedArray: [1, 2, 3],
changedArray2: [3, 2, 1]
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedArray1: [1, 2, 3, 4],
changedArray2: [3, 2, 1]
})
})

it('Should find difference in regexp', () => {
const oldState = {
changedRegexp: /changed/,
unchangedRegexp: /unchanged/
}
const newState = {
changedRegexp: /changedToNewValue/,
unchangedRegexp: /unchanged/
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedRegexp: /changedToNewValue/,
})
})

it('Should find difference in date', () => {
const oldState = {
changedDate: new Date(123),
unchangedDate: new Date(123)
}
const newState = {
changedDate: new Date(1234),
unchangedDate: new Date(123)
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedDate: new Date(1234),
})
})

it('Should find difference in booleans', () => {
const oldState = {
changedBool: true,
unchangedBool: true
}
const newState = {
changedBool: false,
unchangedBool: true
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedBool: false,
})
})

it('Should find difference in numbers', () => {
const oldState = {
changedNumber: 10,
unchangedNumber: 10
}
const newState = {
changedNumber: 9,
unchangedNumber: 10
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedNumber: 9,
})
})

it('Should find difference in strings', () => {
const oldState = {
changedString: 'changed',
unchangedString: 'unchanged'
}
const newState = {
changedString: 'changedToNewValue',
unchangedString: 'unchanged'
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedString: 'changedToNewValue',
})
})

it('Should find new values', () => {
const oldState = {
}
const newState = {
newValue: 10
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
newValue: 10
})
})

it('Should correctly see changes deep in objects', () => {
const oldState = {
changedObject: {
key1: 'unchanged',
key2: {
key1: {
key1: {
key1: false,
key2: true
}
}
},
key3: {
key1: {
key1: {}
},
key2: {
key1: 'abc'
}
},
key4: 50
}
}
const newState = {
changedObject: {
key1: 'unchanged',
key2: {
key1: {
key1: {
key1: true,
key2: true
}
}
},
key3: {
key1: {
key1: {}
},
key2: {
key1: 'abcd'
}
},
key4: 50
}
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
changedObject: {
key2: {
key1: {
key1: {
key1: true,
}
}
},
key3: {
key2: {
key1: 'abcd'
}
},
}
})
})

it('Should find the difference between functions', () => {
const foo = () => {}
const bar = () => {}
const foobar = () => {}

const oldState = {
foo,
bar
}

const newState = {
foo: foobar,
bar
}

const differences = formatStateDifferences(oldState, newState)

expect(differences).toEqual({
foo: foobar
})
})
})
})
10 changes: 9 additions & 1 deletion packages/pinia/src/devtools/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
PINIA_ROOT_ID,
PINIA_ROOT_LABEL,
} from './formatting'
import { isPinia, toastMessage } from './utils'
import { isPinia, toastMessage, formatStateDifferences } from './utils'

// timeline can be paused when directly changing the state
let isTimelineActive = true
Expand Down Expand Up @@ -328,6 +328,8 @@ function addStoreToDevtools(app: DevtoolsApp, store: StoreGeneric) {
store.$onAction(({ after, onError, name, args }) => {
const groupId = runningActionId++

const initialState = JSON.parse(JSON.stringify(toRaw(store.$state)));
Copy link
Member

Choose a reason for hiding this comment

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

We should probably try to use https://developer.mozilla.org/en-US/docs/Web/API/structuredClone if it exists.
It should definitely gracefully fail for circular references and not crash

Copy link
Member

Choose a reason for hiding this comment

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

why are you doing toRaw() here?

Copy link
Author

Choose a reason for hiding this comment

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

toRaw helps with cloning the object. I've tested structuredClone function but it seems it has problems with cloning functions.

I can create my own deepClone function or use the one that lodash provides, but I don't know if you want to have dependencies

Copy link
Member

Choose a reason for hiding this comment

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

there shouldn't be functions in state 😓
Yes, we want to avoid importing lodash

structuredClone have some limitations but it should also be quite fast. You should probably avoid doing toRaw() because we want to unwrap the properties.

I remember Vue devtools had a function for exactly this usecas in their codebase, it might be worth checking it out

Copy link
Author

Choose a reason for hiding this comment

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

From what I can tell, because state is wrapped in a proxy with methods, if I don't make this a raw object structuredClone won't work.

JSON.parse(JSON.stringify) will work without toRaw but the effect will be I think the same.

Could you explain to me why toRaw should be avoided in this situation?
I use this raw object just to copy and display it in devtools.

Copy link
Member

Choose a reason for hiding this comment

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

Because of nested refs


api.addTimelineEvent({
layerId: MUTATIONS_LAYER_ID,
event: {
Expand All @@ -337,6 +339,7 @@ function addStoreToDevtools(app: DevtoolsApp, store: StoreGeneric) {
data: {
store: formatDisplay(store.$id),
action: formatDisplay(name),
initialState,
args,
},
groupId,
Expand All @@ -345,6 +348,9 @@ function addStoreToDevtools(app: DevtoolsApp, store: StoreGeneric) {

after((result) => {
activeAction = undefined
const newState = toRaw(store.$state)
const differences = formatStateDifferences(initialState, newState)

api.addTimelineEvent({
layerId: MUTATIONS_LAYER_ID,
event: {
Expand All @@ -355,6 +361,8 @@ function addStoreToDevtools(app: DevtoolsApp, store: StoreGeneric) {
store: formatDisplay(store.$id),
action: formatDisplay(name),
args,
newState,
differences,
result,
},
groupId,
Expand Down