Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Shallow wrapper #223

Open
wants to merge 67 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
72c6921
Implement shallow wrapper
jeparlefrancais Jul 2, 2019
2b7dde9
Add depth to children wrapper
jeparlefrancais Jul 2, 2019
f57fa89
Support fragments
jeparlefrancais Jul 2, 2019
7ee2939
Filter children prop from shallow.props
jeparlefrancais Jul 9, 2019
45b48da
Add ShallowWrapper:getChildren()
jeparlefrancais Jul 9, 2019
8450335
Split deep equal comparison from assertDeepEqual
jeparlefrancais Jul 19, 2019
a1f914f
Add snapshot serialization
jeparlefrancais Jul 19, 2019
17d8146
Add ref serialization
jeparlefrancais Jul 23, 2019
4183766
Format floats with maximum 7 decimals
jeparlefrancais Jul 25, 2019
830c0b6
Remove Roact.shallow for virtualTree method
jeparlefrancais Jul 25, 2019
39eae67
Rename failed snapshot to new snapshot
jeparlefrancais Jul 25, 2019
16e1ba0
Update snapshot tests
jeparlefrancais Jul 26, 2019
c81d22f
Add snapshot tests
jeparlefrancais Jul 26, 2019
9b09be8
Update snapshot tests
jeparlefrancais Jul 26, 2019
5944bf3
Remove missing lemur events
jeparlefrancais Jul 30, 2019
ae299a5
Pass virtual mounted state in constructor
jeparlefrancais Aug 5, 2019
60b18f2
Rename Snapshot to SnapshotMatcher
jeparlefrancais Aug 5, 2019
2773da6
Rename SnapshotData to Snapshot
jeparlefrancais Aug 5, 2019
edb1ca3
Fix variable shadowing
jeparlefrancais Aug 5, 2019
d73ebf1
Snapshot support table as prop values
jeparlefrancais Aug 5, 2019
f4844ac
Move files into a single folder
jeparlefrancais Aug 7, 2019
a541ec4
Revert VirtualTree
jeparlefrancais Aug 7, 2019
8733795
Rename Snapshot constructor
jeparlefrancais Aug 7, 2019
1b8e6e8
Add API to get snapshot string
jeparlefrancais Aug 7, 2019
0a187d9
Remove children caching on ShallowWrapper
jeparlefrancais Aug 7, 2019
a843b91
Add shallow to global api
jeparlefrancais Aug 7, 2019
caca4ed
Fix format issue
jeparlefrancais Aug 8, 2019
f1da1a2
Rename snapshot to string functions
jeparlefrancais Aug 8, 2019
8dcd096
Fix folder casing
jeparlefrancais Aug 8, 2019
6bd217e
Refactor constraints to be self contained
jeparlefrancais Aug 8, 2019
90a0e40
Add VirtualTree
jeparlefrancais Aug 9, 2019
10c3041
Fix single quotes
jeparlefrancais Aug 9, 2019
846ffd9
Make the ShallowWrapper API strict
jeparlefrancais Aug 9, 2019
764c00a
Update API docs
jeparlefrancais Aug 9, 2019
a36239c
Add snapshots contributing guidelines
jeparlefrancais Aug 9, 2019
7ef84d3
Add plugin to keep snapshots from run to edit mode
jeparlefrancais Aug 9, 2019
dd5bc30
Remove Roact.shallow from docs
jeparlefrancais Aug 12, 2019
3230ad4
Add run-in-roblox docs to contributing guide
jeparlefrancais Aug 12, 2019
8986ce0
Move constraints next to find
jeparlefrancais Aug 13, 2019
9324979
Add shallow rendering page to docs (first pass)
jeparlefrancais Aug 13, 2019
5efa96c
Restructure Shallow Rendering docs
jeparlefrancais Aug 13, 2019
342bbec
Improve snapshot workflow section
jeparlefrancais Aug 13, 2019
750f669
Update snapshot review section
jeparlefrancais Aug 13, 2019
12539ec
Rename ShallowWrapper method toSnapshotString
jeparlefrancais Aug 13, 2019
f1dac37
Remove kind constraint
jeparlefrancais Aug 14, 2019
cfd055a
Improve coverage of validateShallowOptions
jeparlefrancais Aug 14, 2019
4bb8234
Remove unused require
jeparlefrancais Aug 14, 2019
6d677c3
Edit shallow rendering doc to use less second person and tighten up w…
MisterUncloaked Aug 14, 2019
89b3a8e
Merge pull request #2 from MisterUncloaked/shallow-wrapper
jeparlefrancais Aug 14, 2019
7073e0d
Remove ShallowWrapper:getChildren
jeparlefrancais Aug 14, 2019
2764cbc
Remove ShallowWrapper:getChildrenCount()
jeparlefrancais Aug 14, 2019
3f44eab
Rename ShallowWrapper getInstance to getHostObject
jeparlefrancais Aug 15, 2019
0699e75
Add Rect datatype to serializer
jeparlefrancais Aug 16, 2019
d5bcf88
Remove hostKey from top component in snapshots
jeparlefrancais Aug 16, 2019
90010ec
Add depth and find/findUnique examples
jeparlefrancais Aug 16, 2019
5b99367
Throw error when generating snapshots
jeparlefrancais Aug 17, 2019
27db6d9
Update shallow rendering docs
jeparlefrancais Aug 17, 2019
1b1553d
Add snapshots files
jeparlefrancais Aug 19, 2019
ab59379
Merge branch 'master' into shallow-wrapper
jeparlefrancais Aug 19, 2019
13edb92
Remove generated trailing whitespaces in snapshots
jeparlefrancais Aug 20, 2019
ead931b
Move trailing whitespace tests
jeparlefrancais Aug 20, 2019
6dc2133
Remove ElementKind from ShallowWrapper
jeparlefrancais Aug 21, 2019
aec002f
Fix typos and unused parameter
jeparlefrancais Aug 21, 2019
4a83085
Refactor Virtual.mount
jeparlefrancais Aug 21, 2019
01529ba
Simplify IndentedOutput
jeparlefrancais Aug 21, 2019
9f366de
Change getShallowWrapper parameters
jeparlefrancais Aug 22, 2019
780fc4f
Fix shallow rendering docs typo
jeparlefrancais Aug 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 228 additions & 0 deletions src/ShallowWrapper.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
local Children = require(script.Parent.PropMarkers.Children)
local ElementKind = require(script.Parent.ElementKind)
local ElementUtils = require(script.Parent.ElementUtils)
local snapshot = require(script.Parent.snapshot)

local ShallowWrapper = {}
local ShallowWrapperMetatable = {
__index = ShallowWrapper,
}

local function getTypeFromVirtualNode(virtualNode)
local element = virtualNode.currentElement
local kind = ElementKind.of(element)

if kind == ElementKind.Host then
return {
kind = ElementKind.Host,
className = element.component,
}
elseif kind == ElementKind.Function then
return {
kind = ElementKind.Function,
functionComponent = element.component,
}
elseif kind == ElementKind.Stateful then
return {
kind = ElementKind.Stateful,
component = element.component,
}
else
error(('shallow wrapper does not support element of kind %q'):format(tostring(kind)))
Copy link
Contributor

Choose a reason for hiding this comment

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

reminder for us to document the types we support when we get to writing documentation for snapshot testing

end
end

local function findNextVirtualNode(virtualNode, maxDepth)
Copy link
Contributor

Choose a reason for hiding this comment

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

for my own knowledge, what's the role of this function? If we call Shallow.new why can't we assume we passed in the next virtual node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is the thing that gives the illusion of shallow rendering, I have not documented it a lot but if you take a look at the notes repo, checkout the tips page there is a example when you are using depth = 2.

So findNextVirtualNode will get the next virtual node that wraps a host element (a roblox instance). When you allow more depth to the shallow wrapper, it tries to render to roblox instances as much as it can.

I have written a lot of tests to verify the behavior of depth, it's a bit confusing honestly. I've changed the definition a few times before getting this version. It might be something worth reviewing together too if you want

Copy link
Contributor

Choose a reason for hiding this comment

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

Depth might be more intuitive if it still preserves the parent. I tried running some of the tests like stateful-component-children with a depth of 2, and I would have expected CoolComponent to still be in my snapshot, but get to see its children.

local currentDepth = 0
local currentNode = virtualNode
local nextNode = currentNode.children[ElementUtils.UseParentKey]

while currentDepth < maxDepth and nextNode ~= nil do
currentNode = nextNode
nextNode = currentNode.children[ElementUtils.UseParentKey]
currentDepth = currentDepth + 1
end

return currentNode
end

local ContraintFunctions = {
kind = function(virtualNode, expectKind)
return ElementKind.of(virtualNode.currentElement) == expectKind
end,
className = function(virtualNode, className)
local element = virtualNode.currentElement
local isHost = ElementKind.of(element) == ElementKind.Host
return isHost and element.component == className
end,
component = function(virtualNode, expectComponentValue)
return virtualNode.currentElement.component == expectComponentValue
end,
props = function(virtualNode, propSubSet)
local elementProps = virtualNode.currentElement.props

for propKey, propValue in pairs(propSubSet) do
if elementProps[propKey] ~= propValue then
return false
end
end

return true
end,
hostKey = function(virtualNode, expectHostKey)
return virtualNode.hostKey == expectHostKey
end,
}

local function countChildrenOfElement(element)
if ElementKind.of(element) == ElementKind.Fragment then
local count = 0

for _, subElement in pairs(element.elements) do
count = count + countChildrenOfElement(subElement)
end

return count
else
return 1
end
end

local function getChildren(virtualNode, results, maxDepth)
if ElementKind.of(virtualNode.currentElement) == ElementKind.Fragment then
for _, subVirtualNode in pairs(virtualNode.children) do
getChildren(subVirtualNode, results, maxDepth)
end
else
local childWrapper = ShallowWrapper.new(
virtualNode,
maxDepth
)

table.insert(results, childWrapper)
end
end

local function filterProps(props)
Copy link
Contributor

Choose a reason for hiding this comment

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

it could be simpler to just not filter the props in the shallow at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want remove the Roact.Children key from the props when it's there. That is because it makes it easier to assert your props. Intuitively, it matches what you put in the props argument of Roact.createElement.

if props[Children] == nil then
return props
end

local filteredProps = {}

for key, value in pairs(props) do
if key ~= Children then
filteredProps[key] = value
end
end

return filteredProps
end

function ShallowWrapper.new(virtualNode, maxDepth)
virtualNode = findNextVirtualNode(virtualNode, maxDepth)

local wrapper = {
Copy link
Contributor

Choose a reason for hiding this comment

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

to simplify this class, the wrapper could just be the node + the depth, and we could remove some of the mapping we are doing from the virtual node interface to the shallow

_virtualNode = virtualNode,
_childrenMaxDepth = maxDepth - 1,
_virtualNodeChildren = maxDepth == 0 and {} or virtualNode.children,
_shallowChildren = nil,
type = getTypeFromVirtualNode(virtualNode),
props = filterProps(virtualNode.currentElement.props),
hostKey = virtualNode.hostKey,
instance = virtualNode.hostObject,
}

return setmetatable(wrapper, ShallowWrapperMetatable)
end

function ShallowWrapper:childrenCount()
local count = 0

for _, virtualNode in pairs(self._virtualNodeChildren) do
local element = virtualNode.currentElement
count = count + countChildrenOfElement(element)
end

return count
end

function ShallowWrapper:find(constraints)
for constraint in pairs(constraints) do
if not ContraintFunctions[constraint] then
error(('unknown constraint %q'):format(constraint))
end
end

local results = {}
local children = self:getChildren()

for i=1, #children do
local childWrapper = children[i]
if childWrapper:_satisfiesAllContraints(constraints) then
table.insert(results, childWrapper)
end
end

return results
end

function ShallowWrapper:findUnique(constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need convenience methods like this? I'm trying to imagine when developers will want to use this versus relying on snapshot matching.

Copy link
Contributor

Choose a reason for hiding this comment

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

We decided to keep this for cases where we want to remove some scaffolding injected by things like mock providers

local children = self:getChildren()

if constraints == nil then
assert(
#children == 1,
("expect to contain exactly one child, but found %d"):format(#children)
)
return children[1]
end

local constrainedChildren = self:find(constraints)

assert(
#constrainedChildren == 1,
("expect to find only one child, but found %d"):format(#constrainedChildren)
)

return constrainedChildren[1]
end

function ShallowWrapper:getChildren()
if self._shallowChildren then
return self._shallowChildren
jeparlefrancais marked this conversation as resolved.
Show resolved Hide resolved
end

local results = {}

for _, childVirtualNode in pairs(self._virtualNodeChildren) do
getChildren(childVirtualNode, results, self._childrenMaxDepth)
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of using a non member function, this could be a bit simpler:

local childWrapper = ShallowWrapper.new(
	childVirtualNode,
	self._childrenMaxDepth
)
childWrapper:getChildren()
table.insert(results, childWrapper)

this would need the fragment check as well if we got rid of the other function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing I don't like with the member function is that I would have to copy back the elements from the getChildren call into the result table recursively. Do you think it would be better to just go for clarity at this point? I have not benchmark anything yet so I don't have numbers about what is the performance gain

Copy link
Contributor

Choose a reason for hiding this comment

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

Perf is a good point, but I wonder if that concern goes away if we redefine depth such that we're building more deeply nested tables instead of coalescing the children at various levels of the tree to be the shallow children.

end

self._shallowChildren = results
return results
end

function ShallowWrapper:toMatchSnapshot(identifier)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to keep the idea of snapshots separate from the idea of shallows - this API can be the public function I was describing in another comment

assert(typeof(identifier) == "string", "Snapshot identifier must be a string")

local snapshotResult = snapshot(identifier, self)

snapshotResult:match()
end

function ShallowWrapper:_satisfiesAllContraints(constraints)
local virtualNode = self._virtualNode

for constraint, value in pairs(constraints) do
local constraintFunction = ContraintFunctions[constraint]

if not constraintFunction(virtualNode, value) then
return false
end
end

return true
end

return ShallowWrapper
Loading