LUAFDN-1573 - Fix devtools bug during renderer attachment (#385)

* Use 'project' analysis for utility script

* Fix some type stuff

* Add test that covers bugfix

* Fix debugging backend logic errors

bin/ci.sh

@@ -8,7 +8,7 @@ echo "Remove .robloxrc from dependencies"
 find Packages/_Index -name "*.robloxrc" | xargs rm -f
 
 echo "Run static analysis"
-roblox-cli analyze tests.project.json
+roblox-cli analyze --project tests.project.json
 selene --version
 selene --config selene.toml modules/ WorkspaceStatic/
 stylua --version

modules/react-debug-tools/src/ReactDebugHooks.lua

@@ -310,13 +310,11 @@ local function useBinding<T>(initialValue: T): (ReactBinding<T>, ReactBindingUpd
 	local hook = nextHook()
 	local binding = if hook ~= nil
 		then hook.memoizedState
-		else {
+		else ({
 			getValue = function(_self)
 				return initialValue
 			end,
-			-- FIXME Luau: I'd expect luau to complain about a lack of `map`
-			-- field, but it only complains when non-nil and incorrectly typed
-		} :: ReactBinding<T>
+		} :: any) :: ReactBinding<T>
 
 	table.insert(hookLog, {
 		primitive = "Binding",

modules/react-debug-tools/src/__tests__/ReactHooksInspection.spec.lua

@@ -129,7 +129,9 @@ describe("ReactHooksInspection", function()
 						-- ROBLOX deviation END
 						name = "State",
 						subHooks = {},
-						value = "hello",
+						-- ROBLOX deviation START: tell Luau to type this field loosely
+						value = "hello" :: any,
+						-- ROBLOX deviation END
 					},
 					{
 						isStateEditable = false,
@@ -210,7 +212,9 @@ describe("ReactHooksInspection", function()
 		expect(tree).toEqual({
 			{
 				isStateEditable = false,
-				id = nil,
+				-- ROBLOX deviation START: tell Luau to type this field loosely
+				id = nil :: number?,
+				-- ROBLOX deviation END
 				name = "Bar",
 				value = nil,
 				subHooks = {
@@ -273,7 +277,7 @@ describe("ReactHooksInspection", function()
 						isStateEditable = false,
 						-- ROBLOX deviation START: adjust for 1-based indexing
 						-- id = 3,
-						id = 4,
+						id = 4 :: number?,
 						-- ROBLOX deviation END
 						name = "LayoutEffect",
 						-- ROBLOX deviation START: Luau doesn't support mixed arrays

modules/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration.spec.lua

@@ -241,7 +241,9 @@ describe("ReactHooksInspectionIntegration", function()
 				id = 1,
 				-- ROBLOX deviation END
 				name = "State",
-				value = "a",
+				-- ROBLOX deviation START: tell Luau to type this field loosely
+				value = "a" :: any,
+				-- ROBLOX deviation END
 				subHooks = {},
 			},
 			{
@@ -326,7 +328,9 @@ describe("ReactHooksInspectionIntegration", function()
 				id = 1,
 				-- ROBLOX deviation END
 				name = "State",
-				value = "A",
+				-- ROBLOX deviation START: tell Luau to type this field loosely
+				value = "A" :: any,
+				-- ROBLOX deviation END
 				subHooks = {},
 			},
 			{
@@ -563,7 +567,9 @@ describe("ReactHooksInspectionIntegration", function()
 				-- ROBLOX deviation END
 				isStateEditable = false,
 				name = "Transition",
-				value = nil,
+				-- ROBLOX deviation START: tell Luau to type this field loosely
+				value = nil :: any,
+				-- ROBLOX deviation END
 				subHooks = {},
 			},
 			{
@@ -691,7 +697,8 @@ describe("ReactHooksInspectionIntegration", function()
 		function()
 			-- ROBLOX deviation END
 			local function Foo(props)
-				local id = React.unstable_useOpaqueIdentifier()
+				-- ROBLOX FIXME: type this correctly when this is supported
+				local id = (React :: any).unstable_useOpaqueIdentifier()
 				local state = React.useState(function()
 					return "hello"
 					-- ROBLOX deviation START: useState returns 2 values
@@ -779,7 +786,9 @@ describe("ReactHooksInspectionIntegration", function()
 			expect(tree).toEqual({
 				{
 					isStateEditable = false,
-					id = nil,
+					-- ROBLOX deviation START: tell Luau to type this field loosely
+					id = nil :: number?,
+					-- ROBLOX deviation END
 					name = "LabeledValue",
 					-- ROBLOX deviation START: use _G.__DEV__ and cast
 					-- value = if Boolean.toJSBoolean(__DEV__)

modules/react-devtools-shared/src/backend/init.lua

@@ -59,7 +59,7 @@ local function initBackend(hook: DevToolsHook, agent: Agent, global: Object): ()
 		-- ROBLOX deviation: require attach lazily to avoid the require of renderer causing Roact to initialize prematurely.
 		local attach = require(script.renderer).attach
 
-		local rendererInterface = hook.rendererInterfaces[id]
+		local rendererInterface = hook.rendererInterfaces:get(id)
 
 		-- Inject any not-yet-injected renderers (if we didn't reload-and-profile)
 		if rendererInterface == nil then
@@ -74,7 +74,7 @@ local function initBackend(hook: DevToolsHook, agent: Agent, global: Object): ()
 				-- Older react-dom or other unsupported renderer version
 			end
 			if rendererInterface ~= nil then
-				hook.rendererInterfaces[id] = rendererInterface
+				hook.rendererInterfaces:set(id, rendererInterface)
 			end
 		end
 
@@ -92,9 +92,9 @@ local function initBackend(hook: DevToolsHook, agent: Agent, global: Object): ()
 	end
 
 	-- Connect renderers that have already injected themselves.
-	for id, renderer in hook.renderers do
+	hook.renderers:forEach(function(renderer, id)
 		attachRenderer(id, renderer)
-	end
+	end)
 
 	-- Connect any new renderers that injected themselves.
 	table.insert(

modules/react-devtools-shared/src/backend/renderer.lua

@@ -2570,13 +2570,13 @@ exports.attach = function(
 			context = { value = context }
 		end
 
-		local owners = nil
+		local owners: Array<Owner>? = nil
 
 		if _debugOwner then
 			owners = {}
 			local owner: Fiber? = _debugOwner
 			while owner ~= nil do
-				table.insert(owners, {
+				table.insert(owners :: Array<Owner>, {
 					displayName = getDisplayNameForFiber(owner :: Fiber) or "Anonymous",
 					id = getFiberID(getPrimaryFiber(owner :: Fiber)),
 					type = getElementTypeForFiber(owner :: Fiber),
@@ -2604,15 +2604,15 @@ exports.attach = function(
 				hooks = inspectHooksOfFiber(fiber :: Fiber, renderer.currentDispatcherRef)
 			end)
 
-			-- Restore originl console functionality.
+			-- Restore original console functionality.
 			for method, _ in console do
 				pcall(function()
 					console[method] = originalConsoleMethods[method]
 				end)
 			end
 		end
 
-		local rootType = nil
+		local rootType: string? = nil
 		local current = fiber :: Fiber
 
 		while current.return_ ~= nil do
@@ -2659,7 +2659,8 @@ exports.attach = function(
 			-- Inspectable properties.
 			-- TODO Review sanitization approach for the below inspectable values.
 			context = context,
-			hooks = hooks,
+			-- ROBLOX deviation: Luau won't coerce HooksTree to Object
+			hooks = hooks :: any,
 			props = memoizedProps,
 			state = if usesHooks then nil else memoizedState,
 
@@ -2672,7 +2673,7 @@ exports.attach = function(
 			rootType = rootType,
 			rendererPackageName = renderer.rendererPackageName,
 			rendererVersion = renderer.version,
-		} :: InspectedElement
+		}
 	end
 
 	isMostRecentlyInspectedElementCurrent = function(id: number): boolean

modules/react-reconciler/rotriever.toml

@@ -20,3 +20,4 @@ ReactCache = { path = "../react-cache" }
 ReactNoopRenderer = { path = "../react-noop-renderer" }
 ReactTestRenderer = { path = "../react-test-renderer" }
 ReactRoblox = { path = "../react-roblox" }
+ReactDevtoolsShared = { path = "../react-devtools-shared" }

modules/react-reconciler/src/__tests__/ReactFiberDevToolsHook-internal.spec.lua

@@ -19,12 +19,15 @@ local beforeAll = JestGlobals.beforeAll
 local afterAll = JestGlobals.afterAll
 local it = JestGlobals.it
 
-local ReactFiberDevToolsHook
+local Map = require(Packages.LuauPolyfill).Map
+
+local ReactFiberDevToolsHook, ReactDevtoolsShared
 
 beforeEach(function()
 	jest.resetModules()
 
 	ReactFiberDevToolsHook = require(script.Parent.Parent["ReactFiberDevToolsHook.new"])
+	ReactDevtoolsShared = require(Packages.Dev.ReactDevtoolsShared)
 end)
 
 describe("DevTools hook detection", function()
@@ -37,7 +40,7 @@ describe("DevTools hook detection", function()
 		_G.__REACT_DEVTOOLS_GLOBAL_HOOK__ = originalDevtoolsState
 	end)
 
-	local itIfDev = _G.DEV and it or it.skip :: any
+	local itIfDev = if _G.__DEV__ then it else it.skip :: any
 	itIfDev("should log an error when fibers aren't supported", function()
 		_G.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {
 			isDisabled = false,
@@ -53,4 +56,44 @@ describe("DevTools hook detection", function()
 			{ withoutStack = true }
 		)
 	end)
+
+	-- ROBLOX deviation START: verify that renderers are attached correctly
+	it("attaches renderers", function()
+		local renderer123 = {
+			findFiberByHostInstance = function() end,
+		}
+		local renderer456 = {
+			findFiberByHostInstance = function() end,
+		}
+		local hook = {
+			renderers = Map.new({
+				{ 123, renderer123 },
+				{ 456, renderer456 },
+			}),
+			rendererInterfaces = Map.new(),
+			emit = jest.fn(),
+			sub = jest.fn(),
+		}
+		local agent = {
+			addListener = jest.fn(),
+		}
+		_G.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook
+
+		ReactDevtoolsShared.backend.initBackend(hook, agent, {})
+
+		jestExpect(hook.emit).toHaveBeenCalledTimes(3)
+
+		jestExpect(hook.emit).toHaveBeenNthCalledWith(1, "renderer-attached", {
+			id = 123,
+			renderer = renderer123,
+			rendererInterface = jestExpect.anything(),
+		})
+		jestExpect(hook.emit).toHaveBeenNthCalledWith(2, "renderer-attached", {
+			id = 456,
+			renderer = renderer456,
+			rendererInterface = jestExpect.anything(),
+		})
+		jestExpect(hook.emit).toHaveBeenNthCalledWith(3, "react-devtools", agent)
+	end)
+	-- ROBLOX deviation END
 end)