Storybooks manage their own modulel loaders (#35)

# Problem A Roblox internal use case requires initializing a particular singleton module only once. This initialization usually happens in storybooks, which are all put into the same module loader. We need to sandbox each storybook's modules to avoid collisions # Solution The loader is now instantiated and attached to the storybook implicitly in most cases. This improves sandboxing between storybooks. This PR was split from #29 > [!IMPORTANT] > Breaking change: `loadStorybookModule`'s arguments have been reversed. Signature changed from `(ModuleLoader, ModuleScript) -> LoadedStorybook` to `(ModuleScript, ModuleLoader?) -> LoadedStorybook`. Loaders are now optional for the consumer to supply
flipbook-labs · Dec 29, 2024 · 4c5d5cb · 4c5d5cb
1 parent 33b2de9
commit 4c5d5cb
Show file tree

Hide file tree

Showing 13 changed files with 282 additions and 198 deletions.
diff --git a/src/e2e/e2e.spec.luau b/src/e2e/e2e.spec.luau
@@ -37,7 +37,7 @@ describeEach(storybookModules)("e2e %s", function(storybookModule)
 		} :: any
 	) :: ModuleLoader.ModuleLoader
 
-	local storybook = loadStorybookModule(mockModuleLoader, storybookModule)
+	local storybook = loadStorybookModule(storybookModule, mockModuleLoader)
 	local storyModules = findStoryModulesForStorybook(storybook)
 	local container
 
@@ -65,7 +65,7 @@ describeEach(storybookModules)("e2e %s", function(storybookModule)
 		assert(#storyModules >= 2, "storybook must have at least 2 stories")
 
 		for _, storyModule in storyModules do
-			local story = loadStoryModule(mockModuleLoader, storyModule, storybook)
+			local story = loadStoryModule(storyModule, storybook)
 
 			if story.controls then
 				numStoriesWithControls += 1
@@ -78,7 +78,7 @@ describeEach(storybookModules)("e2e %s", function(storybookModule)
 
 	describeEach(storyModules)("%s", function(storyModule)
 		test("basic mount/unmount lifecycle", function()
-			local story = loadStoryModule(mockModuleLoader, storyModule, storybook)
+			local story = loadStoryModule(storyModule, storybook)
 			local lifecycle = render(container, story)
 
 			afterRender()
@@ -91,7 +91,7 @@ describeEach(storybookModules)("e2e %s", function(storybookModule)
 		end)
 
 		test("rerenders on control changes", function()
-			local story = loadStoryModule(mockModuleLoader, storyModule, storybook)
+			local story = loadStoryModule(storyModule, storybook)
 
 			if story.controls then
 				local lifecycle = render(container, story)

diff --git a/src/findStoryModulesForStorybook.spec.luau b/src/findStoryModulesForStorybook.spec.luau
@@ -1,4 +1,5 @@
 local JestGlobals = require("@pkg/JestGlobals")
+local ModuleLoader = require("@pkg/ModuleLoader")
 
 local findStoryModulesForStorybook = require("./findStoryModulesForStorybook")
 local types = require("@root/types")
@@ -27,6 +28,8 @@ test("find all story modules for a storybook", function()
 
 	local storybook: types.LoadedStorybook = {
 		name = "Storybook",
+		loader = ModuleLoader.new(),
+		source = Instance.new("ModuleScript"),
 		storyRoots = {
 			container,
 		},

diff --git a/src/hooks/useStory.luau b/src/hooks/useStory.luau
@@ -1,4 +1,3 @@
-local ModuleLoader = require("@pkg/ModuleLoader")
 local React = require("@pkg/React")
 
 local loadStoryModule = require("@root/loadStoryModule")
@@ -28,11 +27,10 @@ local types = require("@root/types")
 		parent: Instance,
 		storyModule: ModuleScript,
 		storybook: Storybook,
-		loader: ModuleLoader,
 	})
 		local ref = useRef(nil :: Frame?)
 
-		local story = Storyteller.useStory(props.storyModule, props.storybook, props.loader)
+		local story = Storyteller.useStory(props.storyModule, props.storybook)
 
 		useEffect(function()
 			if ref.current then
@@ -54,45 +52,45 @@ local types = require("@root/types")
 	@tag React
 	@tag Story
 	@within Storyteller
-
 ]=]
-local function useStory(
-	module: ModuleScript,
-	storybook: types.LoadedStorybook,
-	loader: ModuleLoader.ModuleLoader
-): (types.LoadedStory<unknown>?, string?)
-	local state, setState = React.useState({} :: {
-		story: types.LoadedStory<unknown>?,
-		err: string?,
-	})
+local function useStory(module: ModuleScript, storybook: types.LoadedStorybook): (types.LoadedStory<unknown>?, string?)
+	local story, setStory = React.useState(nil :: types.LoadedStory<unknown>?)
+	local err, setErr = React.useState(nil :: string?)
 
-	local loadStory = React.useCallback(function()
-		local story
-		local success, err = pcall(function()
-			story = loadStoryModule(loader, module, storybook)
-		end)
+	React.useEffect(function()
+		local function loadStory()
+			local newStory
+			local success, result = pcall(function()
+				newStory = loadStoryModule(module, storybook)
+			end)
+
+			setStory(if success then newStory else nil)
+			setErr(if success then nil else result)
+		end
 
-		setState({
-			story = if success then story else nil,
-			err = if not success then err else nil,
-		})
-	end, { loader, module, storybook } :: { unknown })
+		local connections: { RBXScriptConnection } = {}
 
-	React.useEffect(function()
-		local conn = loader.loadedModuleChanged:Connect(function(other)
-			if other == module then
-				loadStory()
-			end
-		end)
+		table.insert(
+			connections,
+			storybook.loader.loadedModuleChanged:Connect(function(other)
+				if other == module then
+					loadStory()
+				end
+			end)
+		)
+
+		table.insert(connections, module:GetPropertyChangedSignal("Source"):Connect(loadStory))
 
 		loadStory()
 
 		return function()
-			conn:Disconnect()
+			for _, connection in connections do
+				connection:Disconnect()
+			end
 		end
-	end, { module, loadStory, loader } :: { unknown })
+	end, { module, storybook } :: { unknown })
 
-	return state.story, state.err
+	return story, err
 end
 
 return useStory
diff --git a/src/hooks/useStory.spec.luau b/src/hooks/useStory.spec.luau
@@ -14,15 +14,12 @@ local act = ReactRoblox.act
 
 type LoadedStorybook = types.LoadedStorybook
 
-local loader: ModuleLoader.ModuleLoader
 local container: Folder
 local componentModule: ModuleScript
 local storyModule: ModuleScript
-local storybook: types.LoadedStorybook
+local storybook: LoadedStorybook
 
 beforeEach(function()
-	loader = ModuleLoader.new()
-
 	container = Instance.new("Folder")
 
 	componentModule = Instance.new("ModuleScript")
@@ -52,13 +49,15 @@ beforeEach(function()
 
 	storybook = {
 		name = "Storybook",
+		loader = ModuleLoader.new(),
+		source = Instance.new("ModuleScript"),
 		storyRoots = { container },
 	}
 end)
 
 test("loads a story module", function()
 	local get = renderHook(function()
-		return useStory(storyModule, storybook, loader)
+		return useStory(storyModule, storybook)
 	end)
 
 	local story, err = get()
@@ -77,7 +76,7 @@ test("handles story errors", function()
 	storyModule.Source = "syntax error" .. storyModule.Source
 
 	local get = renderHook(function()
-		return useStory(storyModule, storybook, loader)
+		return useStory(storyModule, storybook)
 	end)
 
 	local story, err = get()
@@ -88,13 +87,15 @@ end)
 
 test("re-renders on module changes", function()
 	local get = renderHook(function()
-		return useStory(storyModule, storybook, loader)
+		return useStory(storyModule, storybook)
 	end)
 
-	local story, _err = get()
+	local story, err = get()
 
-	assert(story ~= nil, "story is undefined")
-	expect(story.name).toBe("Sample")
+	expect(err).toBeUndefined()
+	expect(story).toEqual(expect.objectContaining({
+		name = "Sample",
+	}))
 
 	storyModule.Source = [[
 		local Component = require(script.Parent.Component)
@@ -110,15 +111,17 @@ test("re-renders on module changes", function()
 		task.wait()
 	end)
 
-	story, _err = get()
+	story, err = get()
 
-	assert(story ~= nil, "story is undefined")
-	expect(story.name).toBe("Foo")
+	expect(err).toBeUndefined()
+	expect(story).toEqual(expect.objectContaining({
+		name = "Foo",
+	}))
 end)
 
 test("reloads on dependency changes", function()
 	local get = renderHook(function()
-		return useStory(storyModule, storybook, loader)
+		return useStory(storyModule, storybook)
 	end)
 
 	local story = get()