Merge branch 'master' of https://github.com/rojo-rbx/rojo into headless-api

CHANGELOG.md

@@ -2,9 +2,12 @@
 
 ## Unreleased Changes
 * Added headless API for Studio companion plugins. ([#639])
+* Rojo now converts any line endings to LF, preventing spurious diffs when syncing Lua files on Windows ([#854])
+* Fixed Rojo plugin failing to connect when project contains certain unreadable properties ([#848])
 * Added popout diff visualizer for table properties like Attributes and Tags ([#834])
 * Updated Theme to use Studio colors ([#838])
 * Added experimental setting for Auto Connect in playtests ([#840])
+* Fixed http error handling so Rojo can be used in Github Codespaces ([#847])
 * Projects may now specify rules for syncing files as if they had a different file extension. ([#813])
  	This is specified via a new field on project files, `syncRules`:
 
@@ -57,6 +60,10 @@
 [#834]: https://github.com/rojo-rbx/rojo/pull/834
 [#838]: https://github.com/rojo-rbx/rojo/pull/838
 [#840]: https://github.com/rojo-rbx/rojo/pull/840
+[#847]: https://github.com/rojo-rbx/rojo/pull/847
+[#848]: https://github.com/rojo-rbx/rojo/pull/848
+[#854]: https://github.com/rojo-rbx/rojo/pull/854
+
 
 ## [7.4.0] - January 16, 2024
 * Improved the visualization for array properties like Tags ([#829])

crates/memofs/CHANGELOG.md

@@ -2,8 +2,10 @@
 
 ## Unreleased Changes
 * Changed `StdBackend` file watching component to use minimal recursive watches. [#830]
+* Added `Vfs::read_to_string` and `Vfs::read_to_string_lf_normalized` [#854]
 
 [#830]: https://github.com/rojo-rbx/rojo/pull/830
+[#854]: https://github.com/rojo-rbx/rojo/pull/854
 
 ## 0.2.0 (2021-08-23)
 * Updated to `crossbeam-channel` 0.5.1.

crates/memofs/src/lib.rs

@@ -22,9 +22,9 @@ mod noop_backend;
 mod snapshot;
 mod std_backend;
 
-use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex, MutexGuard};
+use std::{io, str};
 
 pub use in_memory_fs::InMemoryFs;
 pub use noop_backend::NoopBackend;
@@ -155,6 +155,24 @@ impl VfsInner {
         Ok(Arc::new(contents))
     }
 
+    fn read_to_string<P: AsRef<Path>>(&mut self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        let contents = self.backend.read(path)?;
+
+        if self.watch_enabled {
+            self.backend.watch(path)?;
+        }
+
+        let contents_str = str::from_utf8(&contents).map_err(|_| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("File was not valid UTF-8: {}", path.display()),
+            )
+        })?;
+
+        Ok(Arc::new(contents_str.into()))
+    }
+
     fn write<P: AsRef<Path>, C: AsRef<[u8]>>(&mut self, path: P, contents: C) -> io::Result<()> {
         let path = path.as_ref();
         let contents = contents.as_ref();
@@ -258,6 +276,33 @@ impl Vfs {
         self.inner.lock().unwrap().read(path)
     }
 
+    /// Read a file from the VFS (or from the underlying backend if it isn't
+    /// resident) into a string.
+    ///
+    /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string].
+    ///
+    /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html
+    #[inline]
+    pub fn read_to_string<P: AsRef<Path>>(&self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        self.inner.lock().unwrap().read_to_string(path)
+    }
+
+    /// Read a file from the VFS (or the underlying backend if it isn't
+    /// resident) into a string, and normalize its line endings to LF.
+    ///
+    /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string], but also performs
+    /// line ending normalization.
+    ///
+    /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html
+    #[inline]
+    pub fn read_to_string_lf_normalized<P: AsRef<Path>>(&self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        let contents = self.inner.lock().unwrap().read_to_string(path)?;
+
+        Ok(contents.lines().collect::<Vec<&str>>().join("\n").into())
+    }
+
     /// Write a file to the VFS and the underlying backend.
     ///
     /// Roughly equivalent to [`std::fs::write`][std::fs::write].

plugin/http/Error.lua

@@ -3,12 +3,12 @@ Error.__index = Error
 
 Error.Kind = {
 	HttpNotEnabled = {
-		message = "Rojo requires HTTP access, which is not enabled.\n" ..
+		message = "Rojo requires HTTP access, which is not enabled.\n"
-			"Check your game settings, located in the 'Home' tab of Studio.",
+			.. "Check your game settings, located in the 'Home' tab of Studio.",
 	},
 	ConnectFailed = {
-		message = "Couldn't connect to the Rojo server.\n" ..
+		message = "Couldn't connect to the Rojo server.\n"
-			"Make sure the server is running — use 'rojo serve' to run it!",
+			.. "Make sure the server is running — use 'rojo serve' to run it!",
 	},
 	Timeout = {
 		message = "HTTP request timed out.",
@@ -63,4 +63,13 @@ function Error.fromRobloxErrorString(message)
 	return Error.new(Error.Kind.Unknown, message)
 end
 
+function Error.fromResponse(response)
+	local lower = (response.body or ""):lower()
+	if response.code == 408 or response.code == 504 or lower:find("timed? ?out") then
+		return Error.new(Error.Kind.Timeout)
+	end
+
+	return Error.new(Error.Kind.Unknown, string.format("%s: %s", tostring(response.code), tostring(response.body)))
+end
+
 return Error

plugin/http/init.lua

@@ -30,8 +30,13 @@ local function performRequest(requestParams)
 			end)
 
 			if success then
-				Log.trace("Request {} success, status code {}", requestId, response.StatusCode)
+				Log.trace("Request {} success, response {:#?}", requestId, response)
-				resolve(HttpResponse.fromRobloxResponse(response))
+				local httpResponse = HttpResponse.fromRobloxResponse(response)
+				if httpResponse:isSuccess() then
+					resolve(httpResponse)
+				else
+					reject(HttpError.fromResponse(httpResponse))
+				end
 			else
 				Log.trace("Request {} failure: {:?}", requestId, response)
 				reject(HttpError.fromRobloxErrorString(response))

plugin/src/App/Components/StringDiffVisualizer/init.lua

@@ -9,6 +9,7 @@ local Log = require(Packages.Log)
 local Highlighter = require(Packages.Highlighter)
 local StringDiff = require(script:FindFirstChild("StringDiff"))
 
+local Timer = require(Plugin.Timer)
 local Theme = require(Plugin.App.Theme)
 
 local CodeLabel = require(Plugin.App.Components.CodeLabel)
@@ -74,6 +75,7 @@ function StringDiffVisualizer:calculateContentSize()
 end
 
 function StringDiffVisualizer:calculateDiffLines()
+	Timer.start("StringDiffVisualizer:calculateDiffLines")
 	local oldString, newString = self.props.oldString, self.props.newString
 
 	-- Diff the two texts
@@ -133,6 +135,7 @@ function StringDiffVisualizer:calculateDiffLines()
 		end
 	end
 
+	Timer.stop()
 	return add, remove
 end
 

plugin/src/App/Components/TableDiffVisualizer/Array.lua

@@ -4,6 +4,7 @@ local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
 
+local Timer = require(Plugin.Timer)
 local Assets = require(Plugin.Assets)
 local Theme = require(Plugin.App.Theme)
 
@@ -21,6 +22,7 @@ function Array:init()
 end
 
 function Array:calculateDiff()
+	Timer.start("Array:calculateDiff")
 	--[[
 		Find the indexes that are added or removed from the array,
 		and display them side by side with gaps for the indexes that
@@ -63,6 +65,7 @@ function Array:calculateDiff()
 		j += 1
 	end
 
+	Timer.stop()
 	return diff
 end
 

plugin/src/App/Components/TableDiffVisualizer/Dictionary.lua

@@ -4,6 +4,7 @@ local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
 
+local Timer = require(Plugin.Timer)
 local Assets = require(Plugin.Assets)
 local Theme = require(Plugin.App.Theme)
 
@@ -21,6 +22,7 @@ function Dictionary:init()
 end
 
 function Dictionary:calculateDiff()
+	Timer.start("Dictionary:calculateDiff")
 	local oldTable, newTable = self.props.oldTable or {}, self.props.newTable or {}
 
 	-- Diff the two tables and find the added keys, removed keys, and changed keys
@@ -59,6 +61,7 @@ function Dictionary:calculateDiff()
 		return a.key < b.key
 	end)
 
+	Timer.stop()
 	return diff
 end
 

plugin/src/App/StatusPages/Confirming.lua

@@ -4,6 +4,8 @@ local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
 
+local Timer = require(Plugin.Timer)
+local PatchTree = require(Plugin.PatchTree)
 local Settings = require(Plugin.Settings)
 local Theme = require(Plugin.App.Theme)
 local TextButton = require(Plugin.App.Components.TextButton)
@@ -23,6 +25,7 @@ function ConfirmingPage:init()
 	self.containerSize, self.setContainerSize = Roact.createBinding(Vector2.new(0, 0))
 
 	self:setState({
+		patchTree = nil,
 		showingStringDiff = false,
 		oldString = "",
 		newString = "",
@@ -30,6 +33,28 @@ function ConfirmingPage:init()
 		oldTable = {},
 		newTable = {},
 	})
+
+	if self.props.confirmData and self.props.confirmData.patch and self.props.confirmData.instanceMap then
+		self:buildPatchTree()
+	end
+end
+
+function ConfirmingPage:didUpdate(prevProps)
+	if prevProps.confirmData ~= self.props.confirmData then
+		self:buildPatchTree()
+	end
+end
+
+function ConfirmingPage:buildPatchTree()
+	Timer.start("ConfirmingPage:buildPatchTree")
+	self:setState({
+		patchTree = PatchTree.build(
+			self.props.confirmData.patch,
+			self.props.confirmData.instanceMap,
+			{ "Property", "Current", "Incoming" }
+		),
+	})
+	Timer.stop()
 end
 
 function ConfirmingPage:render()
@@ -61,9 +86,7 @@ function ConfirmingPage:render()
 				transparency = self.props.transparency,
 				layoutOrder = 3,
 
-				changeListHeaders = { "Property", "Current", "Incoming" },
+				patchTree = self.state.patchTree,
-				patch = self.props.confirmData.patch,
-				instanceMap = self.props.confirmData.instanceMap,
 
 				showStringDiff = function(oldString: string, newString: string)
 					self:setState({

plugin/src/App/StatusPages/Settings/Setting.lua

@@ -124,8 +124,14 @@ function Setting:render()
 				BackgroundTransparency = 1,
 			}, {
 				Name = e("TextLabel", {
-					Text = (if self.props.experimental then '<font color="#FF8E3C">⚠ </font>' else "")
+					Text = (
-						.. self.props.name,
+						if self.props.experimental
+							then '<font color="#FF8E3C">⚠ </font>'
+							elseif
+								self.props.developerDebug
+							then '<font family="rbxasset://fonts/families/Guru.json" color="#35B5FF">⚑ </font>' -- Guru is the only font with the flag emoji
+							else ""
+					) .. self.props.name,
 					Font = Enum.Font.GothamBold,
 					TextSize = 17,
 					TextColor3 = theme.Setting.NameColor,
@@ -140,8 +146,10 @@ function Setting:render()
 				}),
 
 				Description = e("TextLabel", {
-					Text = (if self.props.experimental then '<font color="#FF8E3C">[Experimental] </font>' else "")
+					Text = (if self.props.experimental
-						.. self.props.description,
+						then '<font color="#FF8E3C">[Experimental] </font>'
+						elseif self.props.developerDebug then '<font color="#35B5FF">[Dev Debug] </font>'
+						else "") .. self.props.description,
 					Font = Enum.Font.Gotham,
 					LineHeight = 1.2,
 					TextSize = 14,

plugin/src/App/StatusPages/Settings/init.lua

@@ -83,185 +83,198 @@ function SettingsPage:render()
 	end
 
 	return Theme.with(function(theme)
-		return e(ScrollingFrame, {
+		return Roact.createFragment({
-			size = UDim2.new(1, 0, 1, 0),
-			contentSize = self.contentSize,
-			transparency = self.props.transparency,
-		}, {
 			Navbar = e(Navbar, {
 				onBack = self.props.onBack,
 				transparency = self.props.transparency,
 				layoutOrder = layoutIncrement(),
 			}),
-
+			Content = e(ScrollingFrame, {
-			ShowNotifications = e(Setting, {
+				size = UDim2.new(1, 0, 1, -47),
-				id = "showNotifications",
+				position = UDim2.new(0, 0, 0, 47),
-				name = "Show Notifications",
+				contentSize = self.contentSize,
-				description = "Popup notifications in viewport",
 				transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
+			}, {
-			}),
+				ShowNotifications = e(Setting, {
-
+					id = "showNotifications",
-			SyncReminder = e(Setting, {
+					name = "Show Notifications",
-				id = "syncReminder",
+					description = "Popup notifications in viewport",
-				name = "Sync Reminder",
-				description = "Notify to sync when opening a place that has previously been synced",
-				transparency = self.props.transparency,
-				visible = Settings:getBinding("showNotifications"),
-				layoutOrder = layoutIncrement(),
-			}),
-
-			ConfirmationBehavior = e(Setting, {
-				id = "confirmationBehavior",
-				name = "Confirmation Behavior",
-				description = "When to prompt for confirmation before syncing",
-				transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
-
-				options = confirmationBehaviors,
-			}),
-
-			LargeChangesConfirmationThreshold = e(Setting, {
-				id = "largeChangesConfirmationThreshold",
-				name = "Confirmation Threshold",
-				description = "How many modified instances to be considered a large change",
-				transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
-				visible = Settings:getBinding("confirmationBehavior"):map(function(value)
-					return value == "Large Changes"
-				end),
-				input = e(TextInput, {
-					size = UDim2.new(0, 40, 0, 28),
-					text = Settings:getBinding("largeChangesConfirmationThreshold"):map(function(value)
-						return tostring(value)
-					end),
 					transparency = self.props.transparency,
-					enabled = true,
+					layoutOrder = layoutIncrement(),
-					onEntered = function(text)
-						local number = tonumber(string.match(text, "%d+"))
-						if number then
-							Settings:set("largeChangesConfirmationThreshold", math.clamp(number, 1, 999))
-						else
-							-- Force text back to last valid value
-							Settings:set(
-								"largeChangesConfirmationThreshold",
-								Settings:get("largeChangesConfirmationThreshold")
-							)
-						end
-					end,
 				}),
-			}),
 
-			PlaySounds = e(Setting, {
+				SyncReminder = e(Setting, {
-				id = "playSounds",
+					id = "syncReminder",
-				name = "Play Sounds",
+					name = "Sync Reminder",
-				description = "Toggle sound effects",
+					description = "Notify to sync when opening a place that has previously been synced",
-				transparency = self.props.transparency,
+					transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
+					visible = Settings:getBinding("showNotifications"),
-			}),
+					layoutOrder = layoutIncrement(),
+				}),
 
-			AutoConnectPlaytestServer = e(Setting, {
+				ConfirmationBehavior = e(Setting, {
-				id = "autoConnectPlaytestServer",
+					id = "confirmationBehavior",
-				name = "Auto Connect Playtest Server",
+					name = "Confirmation Behavior",
-				description = "Automatically connect game server to Rojo when playtesting while connected in Edit",
+					description = "When to prompt for confirmation before syncing",
-				experimental = true,
+					transparency = self.props.transparency,
-				transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
-				layoutOrder = layoutIncrement(),
-			}),
 
-			Permissions = e(Setting, {
+					options = confirmationBehaviors,
-				name = "Third Party Permissions",
+				}),
-				description = "Manage permissions for third party plugins",
-				transparency = self.props.transparency,
-				layoutOrder = 6,
-				input = e("TextButton", {
-					Text = "",
-					BackgroundTransparency = 1,
-					Size = UDim2.fromOffset(28, 28),
-					Position = UDim2.fromScale(1, 0.5),
-					AnchorPoint = Vector2.new(1, 0.5),
 
-					[Roact.Event.Activated] = function()
+				LargeChangesConfirmationThreshold = e(Setting, {
-						self.props.onNavigatePermissions()
+					id = "largeChangesConfirmationThreshold",
-					end,
+					name = "Confirmation Threshold",
-				}, {
+					description = "How many modified instances to be considered a large change",
-					Button = e(SlicedImage, {
+					transparency = self.props.transparency,
-						slice = Assets.Slices.RoundedBackground,
+					layoutOrder = layoutIncrement(),
-						color = theme.Checkbox.Active.BackgroundColor,
+					visible = Settings:getBinding("confirmationBehavior"):map(function(value)
+						return value == "Large Changes"
+					end),
+					input = e(TextInput, {
+						size = UDim2.new(0, 40, 0, 28),
+						text = Settings:getBinding("largeChangesConfirmationThreshold"):map(function(value)
+							return tostring(value)
+						end),
 						transparency = self.props.transparency,
-						size = UDim2.new(1, 0, 1, 0),
+						enabled = true,
+						onEntered = function(text)
+							local number = tonumber(string.match(text, "%d+"))
+							if number then
+								Settings:set("largeChangesConfirmationThreshold", math.clamp(number, 1, 999))
+							else
+								-- Force text back to last valid value
+								Settings:set(
+									"largeChangesConfirmationThreshold",
+									Settings:get("largeChangesConfirmationThreshold")
+								)
+							end
+						end,
+					}),
+				}),
+
+				PlaySounds = e(Setting, {
+					id = "playSounds",
+					name = "Play Sounds",
+					description = "Toggle sound effects",
+					transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
+				}),
+
+				AutoConnectPlaytestServer = e(Setting, {
+					id = "autoConnectPlaytestServer",
+					name = "Auto Connect Playtest Server",
+					description = "Automatically connect game server to Rojo when playtesting while connected in Edit",
+					experimental = true,
+					transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
+				}),
+
+				Permissions = e(Setting, {
+					name = "Third Party Permissions",
+					description = "Manage permissions for third party plugins",
+					transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
+					input = e("TextButton", {
+						Text = "",
+						BackgroundTransparency = 1,
+						Size = UDim2.fromOffset(28, 28),
+						Position = UDim2.fromScale(1, 0.5),
+						AnchorPoint = Vector2.new(1, 0.5),
+
+						[Roact.Event.Activated] = function()
+							self.props.onNavigatePermissions()
+						end,
 					}, {
-						Icon = e("ImageLabel", {
+						Button = e(SlicedImage, {
-							Image = Assets.Images.Icons.Expand,
+							slice = Assets.Slices.RoundedBackground,
-							ImageColor3 = theme.Checkbox.Active.IconColor,
+							color = theme.Checkbox.Active.BackgroundColor,
-							ImageTransparency = self.props.transparency,
+							transparency = self.props.transparency,
+							size = UDim2.new(1, 0, 1, 0),
+						}, {
+							Icon = e("ImageLabel", {
+								Image = Assets.Images.Icons.Expand,
+								ImageColor3 = theme.Checkbox.Active.IconColor,
+								ImageTransparency = self.props.transparency,
 
-							Size = UDim2.new(0, 16, 0, 16),
+								Size = UDim2.new(0, 16, 0, 16),
-							Position = UDim2.new(0.5, 0, 0.5, 0),
+								Position = UDim2.new(0.5, 0, 0.5, 0),
-							AnchorPoint = Vector2.new(0.5, 0.5),
+								AnchorPoint = Vector2.new(0.5, 0.5),
 
-							BackgroundTransparency = 1,
+								BackgroundTransparency = 1,
+							}),
 						}),
 					}),
 				}),
-			}),
 
-			OpenScriptsExternally = e(Setting, {
+				OpenScriptsExternally = e(Setting, {
-				id = "openScriptsExternally",
+					id = "openScriptsExternally",
-				name = "Open Scripts Externally",
+					name = "Open Scripts Externally",
-				description = "Attempt to open scripts in an external editor",
+					description = "Attempt to open scripts in an external editor",
-				locked = self.props.syncActive,
+					locked = self.props.syncActive,
-				experimental = true,
+					experimental = true,
-				transparency = self.props.transparency,
+					transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
+					layoutOrder = layoutIncrement(),
-			}),
+				}),
 
-			TwoWaySync = e(Setting, {
+				TwoWaySync = e(Setting, {
-				id = "twoWaySync",
+					id = "twoWaySync",
-				name = "Two-Way Sync",
+					name = "Two-Way Sync",
-				description = "Editing files in Studio will sync them into the filesystem",
+					description = "Editing files in Studio will sync them into the filesystem",
-				locked = self.props.syncActive,
+					locked = self.props.syncActive,
-				experimental = true,
+					experimental = true,
-				transparency = self.props.transparency,
+					transparency = self.props.transparency,
-				layoutOrder = layoutIncrement(),
+					layoutOrder = layoutIncrement(),
-			}),
+				}),
 
-			LogLevel = e(Setting, {
+				LogLevel = e(Setting, {
-				id = "logLevel",
+					id = "logLevel",
-				name = "Log Level",
+					name = "Log Level",
-				description = "Plugin output verbosity level",
+					description = "Plugin output verbosity level",
-				transparency = self.props.transparency,
+					developerDebug = true,
-				layoutOrder = layoutIncrement(),
+					transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
 
-				options = invertedLevels,
+					options = invertedLevels,
-				showReset = Settings:getBinding("logLevel"):map(function(value)
+					showReset = Settings:getBinding("logLevel"):map(function(value)
-					return value ~= "Info"
+						return value ~= "Info"
-				end),
+					end),
-				onReset = function()
+					onReset = function()
-					Settings:set("logLevel", "Info")
+						Settings:set("logLevel", "Info")
-				end,
+					end,
-			}),
+				}),
 
-			TypecheckingEnabled = e(Setting, {
+				TypecheckingEnabled = e(Setting, {
-				id = "typecheckingEnabled",
+					id = "typecheckingEnabled",
-				name = "Typechecking",
+					name = "Typechecking",
-				description = "Toggle typechecking on the API surface",
+					description = "Toggle typechecking on the API surface",
-				transparency = self.props.transparency,
+					developerDebug = true,
-				layoutOrder = layoutIncrement(),
+					transparency = self.props.transparency,
-			}),
+					layoutOrder = layoutIncrement(),
+				}),
 
-			Layout = e("UIListLayout", {
+				TimingLogsEnabled = e(Setting, {
-				FillDirection = Enum.FillDirection.Vertical,
+					id = "timingLogsEnabled",
-				SortOrder = Enum.SortOrder.LayoutOrder,
+					name = "Timing Logs",
+					description = "Toggle logging timing of internal actions for benchmarking Rojo performance",
+					developerDebug = true,
+					transparency = self.props.transparency,
+					layoutOrder = layoutIncrement(),
+				}),
 
-				[Roact.Change.AbsoluteContentSize] = function(object)
+				Layout = e("UIListLayout", {
-					self.setContentSize(object.AbsoluteContentSize)
+					FillDirection = Enum.FillDirection.Vertical,
-				end,
+					SortOrder = Enum.SortOrder.LayoutOrder,
-			}),
 
-			Padding = e("UIPadding", {
+					[Roact.Change.AbsoluteContentSize] = function(object)
-				PaddingLeft = UDim.new(0, 20),
+						self.setContentSize(object.AbsoluteContentSize)
-				PaddingRight = UDim.new(0, 20),
+					end,
+				}),
+
+				Padding = e("UIPadding", {
+					PaddingLeft = UDim.new(0, 20),
+					PaddingRight = UDim.new(0, 20),
+				}),
 			}),
 		})
 	end)

plugin/src/PatchTree.lua

@@ -11,6 +11,7 @@ local Packages = Rojo.Packages
 
 local Log = require(Packages.Log)
 
+local Timer = require(Plugin.Timer)
 local Types = require(Plugin.Types)
 local decodeValue = require(Plugin.Reconciler.decodeValue)
 local getProperty = require(Plugin.Reconciler.getProperty)
@@ -122,6 +123,7 @@ end
 -- props must contain id, and cannot contain children or parentId
 -- other than those three, it can hold anything
 function Tree:addNode(parent, props)
+	Timer.start("Tree:addNode")
 	assert(props.id, "props must contain id")
 
 	parent = parent or "ROOT"
@@ -132,6 +134,7 @@ function Tree:addNode(parent, props)
 		for k, v in props do
 			node[k] = v
 		end
+		Timer.stop()
 		return node
 	end
 
@@ -142,22 +145,26 @@ function Tree:addNode(parent, props)
 	local parentNode = self:getNode(parent)
 	if not parentNode then
 		Log.warn("Failed to create node since parent doesnt exist: {}, {}", parent, props)
+		Timer.stop()
 		return
 	end
 
 	parentNode.children[node.id] = node
 	self.idToNode[node.id] = node
 
+	Timer.stop()
 	return node
 end
 
 -- Given a list of ancestor ids in descending order, builds the nodes for them
 -- using the patch and instanceMap info
 function Tree:buildAncestryNodes(previousId: string?, ancestryIds: { string }, patch, instanceMap)
+	Timer.start("Tree:buildAncestryNodes")
 	-- Build nodes for ancestry by going up the tree
 	previousId = previousId or "ROOT"
 
-	for _, ancestorId in ancestryIds do
+	for i = #ancestryIds, 1, -1 do
+		local ancestorId = ancestryIds[i]
 		local value = instanceMap.fromIds[ancestorId] or patch.added[ancestorId]
 		if not value then
 			Log.warn("Failed to find ancestor object for " .. ancestorId)
@@ -171,6 +178,8 @@ function Tree:buildAncestryNodes(previousId: string?, ancestryIds: { string }, p
 		})
 		previousId = ancestorId
 	end
+
+	Timer.stop()
 end
 
 local PatchTree = {}
@@ -178,10 +187,12 @@ local PatchTree = {}
 -- Builds a new tree from a patch and instanceMap
 -- uses changeListHeaders in node.changeList
 function PatchTree.build(patch, instanceMap, changeListHeaders)
+	Timer.start("PatchTree.build")
 	local tree = Tree.new()
 
 	local knownAncestors = {}
 
+	Timer.start("patch.updated")
 	for _, change in patch.updated do
 		local instance = instanceMap.fromIds[change.id]
 		if not instance then
@@ -189,7 +200,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 		end
 
 		-- Gather ancestors from existing DOM
-		local ancestryIds = {}
+		local ancestryIds, ancestryIndex = {}, 0
 		local parentObject = instance.Parent
 		local parentId = instanceMap.fromInstances[parentObject]
 		local previousId = nil
@@ -200,7 +211,8 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 				break
 			end
 
-			table.insert(ancestryIds, 1, parentId)
+			ancestryIndex += 1
+			ancestryIds[ancestryIndex] = parentId
 			knownAncestors[parentId] = true
 			parentObject = parentObject.Parent
 			parentId = instanceMap.fromInstances[parentObject]
@@ -213,11 +225,38 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 		if next(change.changedProperties) or change.changedName then
 			changeList = {}
 
-			local hintBuffer, i = {}, 0
+			local hintBuffer, hintBufferSize, hintOverflow = table.create(3), 0, 0
+			local changeIndex = 0
 			local function addProp(prop: string, current: any?, incoming: any?, metadata: any?)
-				i += 1
+				changeIndex += 1
-				hintBuffer[i] = prop
+				changeList[changeIndex] = { prop, current, incoming, metadata }
-				changeList[i] = { prop, current, incoming, metadata }
+
+				if hintBufferSize < 3 then
+					hintBufferSize += 1
+					hintBuffer[hintBufferSize] = prop
+					return
+				end
+
+				-- We only want to have 3 hints
+				-- to keep it deterministic, we sort them alphabetically
+
+				-- Either this prop overflows, or it makes another one move to overflow
+				hintOverflow += 1
+
+				-- Shortcut for the common case
+				if hintBuffer[3] <= prop then
+					-- This prop is below the last hint, no need to insert
+					return
+				end
+
+				-- Find the first available spot
+				for i, hintItem in hintBuffer do
+					if prop < hintItem then
+						-- This prop is before the currently selected hint,
+						-- so take its place and then continue to find a spot for the old hint
+						hintBuffer[i], prop = prop, hintBuffer[i]
+					end
+				end
 			end
 
 			-- Gather the changes
@@ -238,18 +277,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 			end
 
 			-- Finalize detail values
-
+			hint = table.concat(hintBuffer, ", ") .. (if hintOverflow == 0 then "" else ", " .. hintOverflow .. " more")
-			-- Trim hint to top 3
-			table.sort(hintBuffer)
-			if #hintBuffer > 3 then
-				hintBuffer = {
-					hintBuffer[1],
-					hintBuffer[2],
-					hintBuffer[3],
-					i - 3 .. " more",
-				}
-			end
-			hint = table.concat(hintBuffer, ", ")
 
 			-- Sort changes and add header
 			table.sort(changeList, function(a, b)
@@ -269,7 +297,9 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 			changeList = changeList,
 		})
 	end
+	Timer.stop()
 
+	Timer.start("patch.removed")
 	for _, idOrInstance in patch.removed do
 		local instance = if Types.RbxId(idOrInstance) then instanceMap.fromIds[idOrInstance] else idOrInstance
 		if not instance then
@@ -311,7 +341,9 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 			instance = instance,
 		})
 	end
+	Timer.stop()
 
+	Timer.start("patch.added")
 	for id, change in patch.added do
 		-- Gather ancestors from existing DOM or future additions
 		local ancestryIds = {}
@@ -350,32 +382,47 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 		if next(change.Properties) then
 			changeList = {}
 
-			local hintBuffer, i = {}, 0
+			local hintBuffer, hintBufferSize, hintOverflow = table.create(3), 0, 0
-			for prop, incoming in change.Properties do
+			local changeIndex = 0
-				i += 1
+			local function addProp(prop: string, incoming: any)
-				hintBuffer[i] = prop
+				changeIndex += 1
+				changeList[changeIndex] = { prop, "N/A", incoming }
 
-				local success, incomingValue = decodeValue(incoming, instanceMap)
+				if hintBufferSize < 3 then
-				if success then
+					hintBufferSize += 1
-					table.insert(changeList, { prop, "N/A", incomingValue })
+					hintBuffer[hintBufferSize] = prop
-				else
+					return
-					table.insert(changeList, { prop, "N/A", select(2, next(incoming)) })
+				end
+
+				-- We only want to have 3 hints
+				-- to keep it deterministic, we sort them alphabetically
+
+				-- Either this prop overflows, or it makes another one move to overflow
+				hintOverflow += 1
+
+				-- Shortcut for the common case
+				if hintBuffer[3] <= prop then
+					-- This prop is below the last hint, no need to insert
+					return
+				end
+
+				-- Find the first available spot
+				for i, hintItem in hintBuffer do
+					if prop < hintItem then
+						-- This prop is before the currently selected hint,
+						-- so take its place and then continue to find a spot for the old hint
+						hintBuffer[i], prop = prop, hintBuffer[i]
+					end
 				end
 			end
 
-			-- Finalize detail values
+			for prop, incoming in change.Properties do
-
+				local success, incomingValue = decodeValue(incoming, instanceMap)
-			-- Trim hint to top 3
+				addProp(prop, if success then incomingValue else select(2, next(incoming)))
-			table.sort(hintBuffer)
-			if #hintBuffer > 3 then
-				hintBuffer = {
-					hintBuffer[1],
-					hintBuffer[2],
-					hintBuffer[3],
-					i - 3 .. " more",
-				}
 			end
-			hint = table.concat(hintBuffer, ", ")
+
+			-- Finalize detail values
+			hint = table.concat(hintBuffer, ", ") .. (if hintOverflow == 0 then "" else ", " .. hintOverflow .. " more")
 
 			-- Sort changes and add header
 			table.sort(changeList, function(a, b)
@@ -395,35 +442,27 @@ function PatchTree.build(patch, instanceMap, changeListHeaders)
 			instance = instanceMap.fromIds[id],
 		})
 	end
+	Timer.stop()
 
+	Timer.stop()
 	return tree
 end
 
--- Creates a deep copy of a tree for immutability purposes in Roact
-function PatchTree.clone(tree)
-	if not tree then
-		return
-	end
-
-	local newTree = Tree.new()
-	tree:forEach(function(node)
-		newTree:addNode(node.parentId, table.clone(node))
-	end)
-
-	return newTree
-end
-
 -- Updates the metadata of a tree with the unapplied patch and currently existing instances
 -- Builds a new tree from the data if one isn't provided
 -- Always returns a new tree for immutability purposes in Roact
 function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch)
+	Timer.start("PatchTree.updateMetadata")
 	if tree then
-		tree = PatchTree.clone(tree)
+		-- A shallow copy is enough for our purposes here since we really only need a new top-level object
+		-- for immutable comparison checks in Roact
+		tree = table.clone(tree)
 	else
 		tree = PatchTree.build(patch, instanceMap)
 	end
 
 	-- Update isWarning metadata
+	Timer.start("isWarning")
 	for _, failedChange in unappliedPatch.updated do
 		local node = tree:getNode(failedChange.id)
 		if not node then
@@ -492,8 +531,10 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch)
 		node.isWarning = true
 		Log.trace("Marked node as warning: {} {}", node.id, node.name)
 	end
+	Timer.stop()
 
 	-- Update if instances exist
+	Timer.start("instanceAncestry")
 	tree:forEach(function(node)
 		if node.instance then
 			if node.instance.Parent == nil and node.instance ~= game then
@@ -509,7 +550,9 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch)
 			end
 		end
 	end)
+	Timer.stop()
 
+	Timer.stop()
 	return tree
 end
 

plugin/src/Reconciler/diff.lua

@@ -177,10 +177,8 @@ local function diff(instanceMap, virtualInstances, rootId)
 
 				if err.kind == Error.UnknownProperty then
 					Log.trace("Skipping unknown property {}.{}", err.details.className, err.details.propertyName)
-				elseif err.kind == Error.UnreadableProperty then
-					Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName)
 				else
-					return false, err
+					Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName)
 				end
 			end
 		end

plugin/src/Reconciler/init.lua

@@ -3,9 +3,14 @@
 	and mutating the Roblox DOM.
 ]]
 
-local Packages = script.Parent.Parent.Packages
+local Rojo = script:FindFirstAncestor("Rojo")
+local Plugin = Rojo.Plugin
+local Packages = Rojo.Packages
+
 local Log = require(Packages.Log)
 
+local Timer = require(Plugin.Timer)
+
 local applyPatch = require(script.applyPatch)
 local hydrate = require(script.hydrate)
 local diff = require(script.diff)
@@ -57,31 +62,55 @@ function Reconciler:hookPostcommit(callback: (patch: any, instanceMap: any, unap
 end
 
 function Reconciler:applyPatch(patch)
+	Timer.start("Reconciler:applyPatch")
+
+	Timer.start("precommitCallbacks")
+	-- Precommit callbacks must be serial in order to obey the contract that
+	-- they execute before commit
 	for _, callback in self.__precommitCallbacks do
 		local success, err = pcall(callback, patch, self.__instanceMap)
 		if not success then
 			Log.warn("Precommit hook errored: {}", err)
 		end
 	end
+	Timer.stop()
 
+	Timer.start("apply")
 	local unappliedPatch = applyPatch(self.__instanceMap, patch)
+	Timer.stop()
 
+	Timer.start("postcommitCallbacks")
+	-- Postcommit callbacks can be called with spawn since regardless of firing order, they are
+	-- guaranteed to be called after the commit
 	for _, callback in self.__postcommitCallbacks do
-		local success, err = pcall(callback, patch, self.__instanceMap, unappliedPatch)
+		task.spawn(function()
-		if not success then
+			local success, err = pcall(callback, patch, self.__instanceMap, unappliedPatch)
-			Log.warn("Postcommit hook errored: {}", err)
+			if not success then
-		end
+				Log.warn("Postcommit hook errored: {}", err)
+			end
+		end)
 	end
+	Timer.stop()
+
+	Timer.stop()
 
 	return unappliedPatch
 end
 
 function Reconciler:hydrate(virtualInstances, rootId, rootInstance)
-	return hydrate(self.__instanceMap, virtualInstances, rootId, rootInstance)
+	Timer.start("Reconciler:hydrate")
+	local result = hydrate(self.__instanceMap, virtualInstances, rootId, rootInstance)
+	Timer.stop()
+
+	return result
 end
 
 function Reconciler:diff(virtualInstances, rootId)
-	return diff(self.__instanceMap, virtualInstances, rootId)
+	Timer.start("Reconciler:diff")
+	local success, result = diff(self.__instanceMap, virtualInstances, rootId)
+	Timer.stop()
+
+	return success, result
 end
 
 return Reconciler

plugin/src/Settings.lua

@@ -20,6 +20,7 @@ local defaultSettings = {
 	playSounds = true,
 	typecheckingEnabled = false,
 	logLevel = "Info",
+	timingLogsEnabled = false,
 	priorEndpoints = {},
 	apiPermissions = {},
 }

plugin/src/Timer.lua

@@ -0,0 +1,57 @@
+local Settings = require(script.Parent.Settings)
+
+local clock = os.clock
+
+local Timer = {
+	_entries = {},
+}
+
+function Timer._start(label)
+	local start = clock()
+	if not label then
+		error("[Rojo-Timer] Timer.start: label is required", 2)
+		return
+	end
+
+	table.insert(Timer._entries, { label, start })
+end
+
+function Timer._stop()
+	local stop = clock()
+
+	local entry = table.remove(Timer._entries)
+	if not entry then
+		error("[Rojo-Timer] Timer.stop: no label to stop", 2)
+		return
+	end
+
+	local label = entry[1]
+	if #Timer._entries > 0 then
+		local priorLabels = {}
+		for _, priorEntry in ipairs(Timer._entries) do
+			table.insert(priorLabels, priorEntry[1])
+		end
+		label = table.concat(priorLabels, "/") .. "/" .. label
+	end
+
+	local start = entry[2]
+	local duration = stop - start
+	print(string.format("[Rojo-Timer] %s took %.3f ms", label, duration * 1000))
+end
+
+-- Replace functions with no-op if not in debug mode
+local function no_op() end
+local function setFunctions(enabled)
+	if enabled then
+		Timer.start = Timer._start
+		Timer.stop = Timer._stop
+	else
+		Timer.start = no_op
+		Timer.stop = no_op
+	end
+end
+
+Settings:onChanged("timingLogsEnabled", setFunctions)
+setFunctions(Settings:get("timingLogsEnabled"))
+
+return Timer

src/snapshot_middleware/lua.rs

@@ -1,6 +1,5 @@
 use std::{collections::HashMap, path::Path, str};
 
-use anyhow::Context;
 use memofs::{IoResultExt, Vfs};
 use rbx_dom_weak::types::Enum;
 
@@ -40,10 +39,8 @@ pub fn snapshot_lua(
         (_, ScriptType::Module) => ("ModuleScript", None),
     };
 
-    let contents = vfs.read(path)?;
+    let contents = vfs.read_to_string_lf_normalized(path)?;
-    let contents_str = str::from_utf8(&contents)
+    let contents_str = contents.as_str();
-        .with_context(|| format!("File was not valid UTF-8: {}", path.display()))?
-        .to_owned();
 
     let mut properties = HashMap::with_capacity(2);
     properties.insert("Source".to_owned(), contents_str.into());

src/snapshot_middleware/txt.rs

@@ -1,6 +1,5 @@
 use std::{path::Path, str};
 
-use anyhow::Context;
 use maplit::hashmap;
 use memofs::{IoResultExt, Vfs};
 
@@ -14,10 +13,8 @@ pub fn snapshot_txt(
     path: &Path,
     name: &str,
 ) -> anyhow::Result<Option<InstanceSnapshot>> {
-    let contents = vfs.read(path)?;
+    let contents = vfs.read_to_string(path)?;
-    let contents_str = str::from_utf8(&contents)
+    let contents_str = contents.as_str();
-        .with_context(|| format!("File was not valid UTF-8: {}", path.display()))?
-        .to_owned();
 
     let properties = hashmap! {
         "Value".to_owned() => contents_str.into(),