-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Draft
KrosFire
wants to merge
6
commits into
vuejs:v2
Choose a base branch
from
KrosFire:ft-better-debug-info
base: v2
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
79c488c
Added functionality for checking differences in state
KrosFire 2fdcec1
Reverted version bump
KrosFire 8d6cd8e
Hotfixes
KrosFire cd361ea
Removed deepCopy
KrosFire 43b6bbc
Implemented fast copy
KrosFire c19379d
Fixed formatting
KrosFire File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
}) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 testedstructuredClone
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 withouttoRaw
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of nested refs