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 37 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
3 changes: 3 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ When submitting a new feature, add tests for all functionality.

We use [LuaCov](https://keplerproject.github.io/luacov) for keeping track of code coverage. We'd like it to be as close to 100% as possible, but it's not always possible. Adding tests just for the purpose of getting coverage isn't useful; we should strive to make only useful tests!

### Snapshots
As part of the test suite, there are tests that generates snapshot files (located under `./RoactSnapshots`). To sync back the generated StringValues produce by these on the file system, we suggest using a tool like [`run-in-roblox`](https://github.com/LPGhatguy/run-in-roblox) to make the workflow as simple as possible. In the case where you only need to sync a few files, you can use the plugin to automatically copy back the generated string values from the run mode into module scripts when going back to edit mode. Then simply copy-paste into the new snapshots into the file-system.

## Release Checklist
When releasing a new version of Roact, do these things:

Expand Down
41 changes: 41 additions & 0 deletions SnapshotsPlugin/EditModeMain.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local Settings = require(script.Parent.Settings)

local function SyncSnapshots(newSnapshots)
local snapshotsFolder = ReplicatedStorage:FindFirstChild(Settings.SnapshotFolderName)

if not snapshotsFolder then
snapshotsFolder = Instance.new("Folder")
snapshotsFolder.Name = Settings.SnapshotFolderName
snapshotsFolder.Parent = ReplicatedStorage
end

for name, value in pairs(newSnapshots) do
local snapshot = Instance.new("ModuleScript")
snapshot.Name = name
snapshot.Source = value
snapshot.Parent = snapshotsFolder
end
end

local function PluginEditMode(plugin)
local isPluginDeactivated = false

plugin.Deactivation:Connect(function()
isPluginDeactivated = true
end)

while not isPluginDeactivated do
local newSnapshots = plugin:GetSetting(Settings.PluginSettingName)

if newSnapshots then
SyncSnapshots(newSnapshots)
plugin:SetSetting(Settings.PluginSettingName, false)
end

wait(Settings.SyncDelay)
end
end

return PluginEditMode
25 changes: 25 additions & 0 deletions SnapshotsPlugin/RunModeMain.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local Settings = require(script.Parent.Settings)

local function PluginRunMode(plugin)
plugin.Unloading:Connect(function()
local snapshotsFolder = ReplicatedStorage:FindFirstChild(Settings.SnapshotFolderName)

local newSnapshots = {}

if not snapshotsFolder then
return
end

for _, snapshot in pairs(snapshotsFolder:GetChildren()) do
if snapshot:IsA("StringValue") then
newSnapshots[snapshot.Name] = snapshot.Value
end
end

plugin:SetSetting(Settings.PluginSettingName, newSnapshots)
end)
end

return PluginRunMode
17 changes: 17 additions & 0 deletions SnapshotsPlugin/Settings.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
local function IndexError(_, key)
local message = ("%q (%s) is not a valid member of Settings"):format(
tostring(key),
typeof(key)
)

error(message, 2)
end

return setmetatable({
SnapshotFolderName = "RoactSnapshots",
PluginSettingName = "NewRoactSnapshots",
SyncDelay = 1,
}, {
__index = IndexError,
__newindex = IndexError,
})
12 changes: 12 additions & 0 deletions SnapshotsPlugin/init.server.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
local RunService = game:GetService("RunService")

local EditModeMain = require(script.EditModeMain)
local RunModeMain = require(script.RunModeMain)

if RunService:IsEdit() then
EditModeMain(plugin)
else
if RunService:IsClient() then
RunModeMain(plugin)
end
end
167 changes: 161 additions & 6 deletions docs/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,21 @@ Creates a new Roact fragment with the provided table of elements. Fragments allo

### Roact.mount
```
Roact.mount(element, [parent, [key]]) -> RoactTree
Roact.mount(element, [parent, [key]]) -> VirtualTree
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if I agree with changing the terminology here. Ideally, we should expose as little internal vocabulary as possible in order to define what snapshots are and what they do.

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 can switch it back to RoactTree, it's just that the Type we are going to expose in #230 is called VirtualTree. So maybe it could be renamed in that other PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good question! I didn't realize we were exposing it in that PR. We should probably discuss this with @LPGhatguy.

My thoughts are still that VirtualTree probably means less to users, but maybe there's another better alternative too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree that the term VirtualTree might be questionable to users, we should probably solves this in #230 if it's about to be merged first.

```

!!! info
`Roact.mount` is also available via the deprecated alias `Roact.reify`. It will be removed in a future release.

Creates a Roblox Instance given a Roact element, and optionally a `parent` to put it in, and a `key` to use as the instance's `Name`.

The result is a `RoactTree`, which is an opaque handle that represents a tree of components owned by Roact. You can pass this to APIs like `Roact.unmount`. It'll also be used for future debugging APIs.
The result is a `VirtualTree`, which is an opaque handle that represents a tree of components owned by Roact. You can pass this to APIs like `Roact.unmount`. It'll also be used for future debugging APIs.

---

### Roact.update
```
Roact.update(tree, element) -> RoactTree
Roact.update(tree, element) -> VirtualTree
```

!!! info
Expand All @@ -56,7 +56,7 @@ Updates an existing instance handle with a new element, returning a new handle.

`update` can be used to change the props of a component instance created with `mount` and is useful for putting Roact content into non-Roact applications.

As of Roact 1.0, the returned `RoactTree` object will always be the same value as the one passed in.
As of Roact 1.0, the returned `VirtualTree` object will always be the same value as the one passed in.

---

Expand All @@ -68,7 +68,7 @@ Roact.unmount(tree) -> void
!!! info
`Roact.unmount` is also available via the deprecated alias `Roact.teardown`. It will be removed in a future release.

Destroys the given `RoactTree` and all of its descendants. Does not operate on a Roblox Instance -- this must be given a handle that was returned by `Roact.mount`.
Destroys the given `VirtualTree` and all of its descendants. Does not operate on a Roblox Instance -- this must be given a handle that was returned by `Roact.mount`.

---

Expand Down Expand Up @@ -608,4 +608,159 @@ end
As with `setState`, you can set use the constant `Roact.None` to remove a field from the state.

!!! note
`getDerivedStateFromProps` is a *static* lifecycle method. It does not have access to `self`, and must be a pure function.
`getDerivedStateFromProps` is a *static* lifecycle method. It does not have access to `self`, and must be a pure function.

---

## ShallowWrapper

### Fields

#### type
```
type: {
kind: ElementKind
Copy link
Contributor

Choose a reason for hiding this comment

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

like with #227 we might want a mirror and not expose the ElementKind symbols directly, so Elements cannot be impersonated.

}
```

The type dictionary always has the `kind` field that tell the component type. Additionally, depending of the kind of component, other information can be included.

| kind | fields | description |
| --- | --- | --- |
| Host | className: string | the ClassName of the instance |
| Function | functionComponent: function | the function that renders the element |
| Stateful | component: table | the class-like table used to render the element |
ZoteTheMighty marked this conversation as resolved.
Show resolved Hide resolved

---

#### props
The props of the ShallowWrapper.

!!! note
This dictionary will not contain the `Roact.Children` prop. To obtain the children elements wrapped into a ShallowWrapper, use the method `getChildren()`
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be simpler to just populate children as a property of ShallowWrapper when we call new? I'm trying to think of when you want a ShallowWrapper but wouldn't call getChildren()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I just make a children field and remove getChildren and getChildrenCount?


---

#### hostKey
The `hostKey` that is used to map the element to it's parent.

---

### Methods

#### childrenCount
```
childrenCount() -> number
```
Returns the amount of children that the current ShallowWrapper contains.

---

#### find
```
find([constraints]) -> list[ShallowWrapper]
```
When a dictionary of constraints is provided, the function will filter the children that do not satisfy all given constraints. Otherwise, as `getChildren` do, it returns a list of all children wrapped into ShallowWrappers.

---

#### findUnique
```
findUnique([constraints]) -> ShallowWrapper
```
Similar to `find`, this method will assert that only one child satisfies the given constraints, or in the case where none is provided, will assert that there is simply only one child.

---

#### getChildren
```
getChildren() -> list[ShallowWrapper]
```
Returns a list of all children wrapped into ShallowWrappers.

---

#### getInstance
```
getInstance() -> Instance or nil
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 getInstance? tostring + matchsnapshot + supporting methods to query the public fields of the shallow tree should be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For more robust tests it can be interesting to directly assert on the instance. For example you can assert for a specific value of AbsoluteSize or AbsolutePosition. It could also useful if you want to test behavior from lifecycle methods that act on a ref.

Copy link
Contributor

Choose a reason for hiding this comment

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

In such a test, could you just use a ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is already a Ref, yes because you can access it through the props. Otherwise I don't think so

```
Returns the instance object associated with the ShallowWrapper. It can return `nil` if the component wrapped by the ShallowWrapper does not render an instance, but rather another component. Here is an example:

```lua
local function CoolComponent(props)
return Roact.createElement("TextLabel")
end

local function MainComponent(props)
return Roact.createElement("Frame", {}, {
Cool = Roact.createElement(CoolComponent),
})
end
```

If we shallow-render the `MainComponent`, the default depth will not render the CoolComponent.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an explanation of how render depth works, not an explanation of what getHostObject does

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 wanted to explain when getHostObject can return nil or not, but yep it does include talking a bit about depth... Should I remove it or not?


```lua
local element = Roact.createElement(MainComponent)

local tree = Roact.mount(element)

local wrapper = tree:getShallowWrapper()

print(wrapper:getInstance() == nil) -- prints false

local coolWrapper = wrapper:findUnique()

print(coolWrapper:getInstance() == nil) -- prints true
```

---

#### matchSnapshot
```
matchSnapshot(identifier)
```
If no previous snapshot with the given identifier exists, it will create a new StringValue instance that will contain Lua code representing the current ShallowWrapper. When an existing snapshot is found (a ModuleScript named as the provided identifier), it will require the ModuleScript and load the data from it. Then, if the loaded data is different from the current ShallowWrapper, an error will be thrown.

!!! note
As mentionned, `matchSnapshot` will create a StringValue, named like the given identifier, in which the generated lua code will be assigned to the Value property. When these values are generated in Studio during run mode, it's important to copy back the values and convert them into ModuleScripts.

---

#### snapshotToString
```
snapshotToString() -> string
```
Returns the string source of the snapshot. Useful for debugging purposes.

---

##### Constraints
Constraints are passed through a dictionary that maps a constraint name to it's value.

| name | value type | description |
| --- | --- | --- |
| kind | ElementKind | Filters with the ElementKind of the rendered elements |
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 API references in this file for ElementKind since we are exposing it now as public for these constraint functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes if we don't expose ElementKind we should remove that!

| className | string | Filters children that renders to host instance of the given class name |
| component | string, function or table | Filter children from their components, by finding the functional component original function, the sub-class table of Roact.Component or from the class name of the instance rendered |
| props | dictionary | Filters elements that contains at least the given set of props |
| hostKey | string | Filters elements by the host key used |

---

## VirtualTree

### Methods

#### getShallowWrapper
```
getShallowWrapper(options) -> ShallowWrapper
```
Options:
```lua
{
depth: number -- default to 1
}
```

Wraps the current tree into a ShallowWrapper.
6 changes: 6 additions & 0 deletions snapshots-plugin.project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "Snapshots Plugin",
"tree": {
"$path": "SnapshotsPlugin"
}
}
Loading