From c2a3b0416d59bd2f5757912f6cc16f7106968552 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 5 Nov 2024 16:56:37 -0800 Subject: [PATCH 01/38] Work from the day --- foreman.toml | 10 ++ src/createRendererForStory.luau | 6 + .../storybooks/RobloxInternal.storybook.luau | 15 +++ src/hooks/useStorybooks.luau | 31 ++++- src/migrateLegacyPackages.luau | 2 + .../createRobloxInternalRenderer.luau | 60 ++++++++++ .../createRobloxInternalRenderer.spec.luau | 107 ++++++++++++++++++ src/roblox-internal/isReact.luau | 18 +++ src/roblox-internal/isReact.spec.luau | 26 +++++ src/types.luau | 14 +++ wally.toml | 1 + 11 files changed, 285 insertions(+), 5 deletions(-) create mode 100644 foreman.toml create mode 100644 src/e2e/storybooks/RobloxInternal.storybook.luau create mode 100644 src/renderers/createRobloxInternalRenderer.luau create mode 100644 src/renderers/createRobloxInternalRenderer.spec.luau create mode 100644 src/roblox-internal/isReact.luau create mode 100644 src/roblox-internal/isReact.spec.luau diff --git a/foreman.toml b/foreman.toml new file mode 100644 index 0000000..c732467 --- /dev/null +++ b/foreman.toml @@ -0,0 +1,10 @@ +[tools] +rojo = { source = "rojo-rbx/rojo", version = "7.4.3" } +selene = { source = "Kampfkarren/selene", version = "0.27.1" } +StyLua = { source = "JohnnyMorganz/StyLua", version = "0.20.0" } +wally = { source = "UpliftGames/wally", version = "0.3.2" } +luau-lsp = { source = "JohnnyMorganz/luau-lsp", version = "1.34.0" } +wally-package-types = { source = "JohnnyMorganz/wally-package-types", version = "1.3.2" } +lune = { source = "lune-org/lune", version = "0.8.7" } +darklua = { source = "seaofvoices/darklua", version = "0.13.1" } +run-in-roblox = { source = "rojo-rbx/run-in-roblox", version = "0.3.0" } diff --git a/src/createRendererForStory.luau b/src/createRendererForStory.luau index fcaf9c1..fed4a6d 100644 --- a/src/createRendererForStory.luau +++ b/src/createRendererForStory.luau @@ -2,6 +2,8 @@ local createFusionRenderer = require("@root/renderers/createFusionRenderer") local createManualRenderer = require("@root/renderers/createManualRenderer") local createReactRenderer = require("@root/renderers/createReactRenderer") local createRoactRenderer = require("@root/renderers/createRoactRenderer") +local createRobloxInternalRenderer = require("@root/renderers/createRobloxInternalRenderer") +local isReact = require("@root/roblox-internal/isReact") local types = require("@root/types") type Story = types.Story @@ -21,6 +23,10 @@ type StoryRenderer = types.StoryRenderer local function createRendererForStory(story: Story): StoryRenderer local packages = if story.packages then story.packages else story.storybook.packages if packages then + if isReact(packages.Roact) or packages.React and packages.ReactRoblox then + return createRobloxInternalRenderer(packages) + end + if packages.Roact then return createRoactRenderer(packages) elseif packages.React and packages.ReactRoblox then diff --git a/src/e2e/storybooks/RobloxInternal.storybook.luau b/src/e2e/storybooks/RobloxInternal.storybook.luau new file mode 100644 index 0000000..9a9780c --- /dev/null +++ b/src/e2e/storybooks/RobloxInternal.storybook.luau @@ -0,0 +1,15 @@ +local RoactCompat = require("@pkg/RoactCompat") + +return { + roact = RoactCompat, + storyRoots = { + script.Parent.stories.react, + }, + mapStory = function(story) + return function(storyProps) + return RoactCompat.createElement("Frame", { + BackgroundColor3 = Color3.fromRGB(255, 0, 0), + }, RoactCompat.createElement(story, storyProps)) + end + end, +} diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 909e8db..ec7a58f 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -5,6 +5,11 @@ local findStorybookModules = require("@root/findStorybookModules") local loadStorybookModule = require("@root/loadStorybookModule") local types = require("@root/types") +type UnavailableStorybook = { + problem: string, + storybook: types.Storybook, +} + --[=[ Performs all the discovery and loading of Storybook modules that would normally be done via individual API members. @@ -60,25 +65,38 @@ local types = require("@root/types") @tag Storybook @within Storyteller ]=] -local function useStorybooks(parent: Instance, loader: ModuleLoader.ModuleLoader): { types.Storybook } +local function useStorybooks( + parent: Instance, + loader: ModuleLoader.ModuleLoader +): { types.Storybook | UnavailableStorybook } local storybooks, setStorybooks = React.useState({}) + local unavailableStorybooks, setUnavailableStorybooks = React.useState({} :: { UnavailableStorybook }) local loadStorybooks = React.useCallback(function() local newStorybooks = {} + local newUnavailableStorybooks: { UnavailableStorybook } = {} for _, storybookModule in findStorybookModules(parent) do + local storybook: types.Storybook? local success, result = pcall(function() - return loadStorybookModule(loader, storybookModule) + storybook = loadStorybookModule(loader, storybookModule) end) if success then - table.insert(newStorybooks, result) + table.insert(newStorybooks, storybook) else - warn(result) + table.insert(newUnavailableStorybooks, { + problem = result, + storybook = { + name = storybookModule.Name, + storyRoots = {}, + }, + }) end end setStorybooks(newStorybooks) + setUnavailableStorybooks(newUnavailableStorybooks) end, { parent, loader } :: { unknown }) React.useEffect(function() @@ -95,7 +113,10 @@ local function useStorybooks(parent: Instance, loader: ModuleLoader.ModuleLoader end end, { loadStorybooks, loader } :: { unknown }) - return storybooks + return { + available = storybooks, + unavailable = unavailableStorybooks, + } end return useStorybooks diff --git a/src/migrateLegacyPackages.luau b/src/migrateLegacyPackages.luau index 85fbb40..5b22843 100644 --- a/src/migrateLegacyPackages.luau +++ b/src/migrateLegacyPackages.luau @@ -1,3 +1,4 @@ +local isReact = require("@root/roblox-internal/isReact") local types = require("@root/types") type StoryPackages = types.StoryPackages @@ -6,6 +7,7 @@ local function migrateLegacyPackages(storyOrStorybook: { [string]: any }): Story if storyOrStorybook.roact or storyOrStorybook.react or storyOrStorybook.reactRoblox then return { Roact = storyOrStorybook.roact, + RoactCompat = if isReact(storyOrStorybook.roact) then storyOrStorybook.roact else nil, React = storyOrStorybook.react, ReactRoblox = storyOrStorybook.reactRoblox, } diff --git a/src/renderers/createRobloxInternalRenderer.luau b/src/renderers/createRobloxInternalRenderer.luau new file mode 100644 index 0000000..bc2e848 --- /dev/null +++ b/src/renderers/createRobloxInternalRenderer.luau @@ -0,0 +1,60 @@ +local types = require("@root/types") + +type StoryRenderer = types.StoryRenderer + +type Packages = { + React: any, + RoactCompat: any, + ReactRoblox: any, +} + +local function isReactElement(maybeElement: any): boolean + if typeof(maybeElement) == "table" then + return maybeElement["$$typeof"] and maybeElement.props and maybeElement.type + end + return false +end + +local function createRobloxInternalRenderer(packages: Packages): StoryRenderer + local React = packages.React + local RoactCompat = packages.RoactCompat + local ReactRoblox = packages.ReactRoblox + + local root + local currentStory + + local function reactRender(story: types.Story, props) + local element + if isReactElement(story.story) then + element = story.story + else + element = React.createElement(story.story, props) + end + + currentStory = story + root:render(element) + end + + local function mount(container, story, props) + root = ReactRoblox.createRoot(container) + reactRender(story, props) + end + + local function update(props) + if currentStory then + reactRender(currentStory, props) + end + end + + local function unmount() + root:unmount() + end + + return { + mount = mount, + update = update, + unmount = unmount, + } +end + +return createRobloxInternalRenderer diff --git a/src/renderers/createRobloxInternalRenderer.spec.luau b/src/renderers/createRobloxInternalRenderer.spec.luau new file mode 100644 index 0000000..1390b80 --- /dev/null +++ b/src/renderers/createRobloxInternalRenderer.spec.luau @@ -0,0 +1,107 @@ +local Fusion = require("@pkg/Fusion") +local JestGlobals = require("@pkg/JestGlobals") + +local createStory = require("@root/test-utils/createStory") +local render = require("@root/render") + +local beforeEach = JestGlobals.beforeEach +local expect = JestGlobals.expect +local test = JestGlobals.test + +local New = Fusion.New +local Computed = Fusion.Computed +type StateObject = Fusion.StateObject + +local function TextStory() + return New("TextLabel")({ + Text = "Hello World", + }) +end + +local function ButtonStory(props: { + controls: { + isDisabled: StateObject, + }, +}) + return New("TextButton")({ + Text = Computed(function() + return if props.controls.isDisabled:get() then "Disabled" else "Enabled" + end), + }) +end + +local function createFusionStory(element) + return createStory(element, { + Fusion = Fusion, + }) +end + +local container + +beforeEach(function() + container = Instance.new("Folder") +end) + +test("render a Fusion component", function() + local story = createFusionStory(TextStory) + + render(container, story) + + expect(#container:GetChildren()).toBe(1) + expect(container:FindFirstChildWhichIsA("TextLabel")).toBeDefined() +end) + +test("unmount a Fusion component", function() + local story = createStory(TextStory) + local lifecycle = render(container, story) + + expect(#container:GetChildren()).toBe(1) + + lifecycle.unmount() + + expect(#container:GetChildren()).toBe(0) +end) + +test("controls are transformed into Values", function() + local story = createFusionStory(ButtonStory) + story.controls = { + isDisabled = true, + } + + render(container, story) + + local element = container:FindFirstChildWhichIsA("TextButton") + assert(element, "no element found") + expect(element.Text).toBe("Disabled") +end) + +test("update the component on arg changes", function() + expect(#container:GetChildren()).toBe(0) + + local story = createFusionStory(ButtonStory) + story.controls = { + isDisabled = true, + } + + local lifecycle = render(container, story) + + expect(#container:GetChildren()).toBe(1) + + local element = container:FindFirstChildWhichIsA("TextButton") + assert(element, "no element found") + expect(element.Text).toBe("Disabled") + + lifecycle.update({ + isDisabled = false, + }) + + expect(#container:GetChildren()).toBe(1) + + local prevElement = element + element = container:FindFirstChildWhichIsA("TextButton") + expect(element).toBe(prevElement) + + task.wait() + + expect(element.Text).toBe("Enabled") +end) diff --git a/src/roblox-internal/isReact.luau b/src/roblox-internal/isReact.luau new file mode 100644 index 0000000..3ea5f5e --- /dev/null +++ b/src/roblox-internal/isReact.luau @@ -0,0 +1,18 @@ +--[[ + This function is used to differentiate between React and legacy Roact only + for the consumption of Roblox Internal storybooks. +]] +local function isReact(maybeReact: any): boolean + if maybeReact.Ref == "ref" and maybeReact.Children == "children" then + return true + -- Legacy Roact is a strict table, and indexing it directly throws an error + -- if a member doesn't exist. To get around this we use rawget to check if + -- it's React + elseif rawget(maybeReact, "useState") ~= nil then + return true + else + return false + end +end + +return isReact diff --git a/src/roblox-internal/isReact.spec.luau b/src/roblox-internal/isReact.spec.luau new file mode 100644 index 0000000..5954a07 --- /dev/null +++ b/src/roblox-internal/isReact.spec.luau @@ -0,0 +1,26 @@ +local JestGlobals = require("@pkg/JestGlobals") +local React = require("@pkg/React") +local ReactRoblox = require("@pkg/ReactRoblox") +local Roact = require("@pkg/Roact") +local RoactCompat = require("@pkg/RoactCompat") + +local isReact = require("./isReact") + +local test = JestGlobals.test +local expect = JestGlobals.expect + +test("React is React", function() + expect(isReact(React)).toBeTruthy() +end) + +test("RoactCompat is not React", function() + expect(isReact(RoactCompat)).toBeTruthy() +end) + +test("ReactRoblox is not React", function() + expect(isReact(ReactRoblox)).toBeTruthy() +end) + +test("legacy Roact is not React", function() + expect(isReact(Roact)).toBeTruthy() +end) diff --git a/src/types.luau b/src/types.luau index f0fa05b..ed81816 100644 --- a/src/types.luau +++ b/src/types.luau @@ -31,11 +31,25 @@ export type StoryPackages = { [string]: any, } +type MapStory = (props: StoryProps) -> any + +type RobloxInternalStorybook = { + storyRoots: { Instance }, + + name: string?, + mapStory: ((story: any) -> (props: StoryProps) -> any)?, + + roact: any, + react: any, + reactRoblox: any, +} + export type Storybook = { storyRoots: { Instance }, name: string?, packages: StoryPackages?, + -- mapStory: ((story: Story) -> Story)?, } types.IStorybook = t.interface({ storyRoots = t.array(t.Instance), diff --git a/wally.toml b/wally.toml index c22d27e..d0a0406 100644 --- a/wally.toml +++ b/wally.toml @@ -22,6 +22,7 @@ React = "jsdotlua/react@17.0.2" # [dev-dependencies] ReactRoblox = "jsdotlua/react-roblox@17.0.2" +RoactCompat = "jsdotlua/roact-compat@17.0.2" Roact = "roblox/roact@1.4.4" Fusion = "elttob/fusion@0.2.0" Jest = "jsdotlua/jest@3.6.1-rc.2" From 7ec177d334e7e106279cf67523d37bbb543768f3 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 13:48:01 -0800 Subject: [PATCH 02/38] Fix types --- src/hooks/useStorybooks.luau | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index ec7a58f..bc739b9 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -68,7 +68,10 @@ type UnavailableStorybook = { local function useStorybooks( parent: Instance, loader: ModuleLoader.ModuleLoader -): { types.Storybook | UnavailableStorybook } +): { + available: { types.Storybook }, + unavailable: { UnavailableStorybook }, +} local storybooks, setStorybooks = React.useState({}) local unavailableStorybooks, setUnavailableStorybooks = React.useState({} :: { UnavailableStorybook }) From 4dfd89e638378faad5984cd6772d82aeaf797f0f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 13:48:23 -0800 Subject: [PATCH 03/38] Place raw errors on the next line --- src/loadStoryModule.luau | 4 ++-- src/loadStorybookModule.luau | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index a0ad05d..0a0f032 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -38,7 +38,7 @@ local function loadStoryModule( end) if not success then - error(`Failed to load story {storyModule:GetFullName()}. Error: {result})`) + error(`Failed to load story {storyModule:GetFullName()}:\n{result})`) end -- Support for Hoarcekat stories @@ -56,7 +56,7 @@ local function loadStoryModule( if not isValid then error( `Story is malformed. Check the source of {storyModule:GetFullName()} and make sure its properties are ` - .. `correct. Error: {message}` + .. `correct.\nerr: {message}` ) end diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index c23fdd2..47ce996 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -23,11 +23,11 @@ local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: Mo return loader:require(module) end) - assert(wasRequired, `failed to load storybook`) + assert(wasRequired, `failed to load storybook:\n{result}`) local isStorybook, message = types.IStorybook(result) - assert(isStorybook, `failed to load storybook {module:GetFullName()}: {message}`) + assert(isStorybook, `failed to load storybook {module:GetFullName()}:\n{message}`) if not result.packages then result.packages = migrateLegacyPackages(result) From 922c3c365dc4044a377a05f116183bd4865309be Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 16:08:09 -0800 Subject: [PATCH 04/38] Support "group" field on storybooks --- .darklua.json | 7 ++++++- src/loadStorybookModule.luau | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.darklua.json b/.darklua.json index 183d337..da21440 100644 --- a/.darklua.json +++ b/.darklua.json @@ -15,6 +15,11 @@ "indexing_style": "property" } }, + { + "rule": "inject_global_value", + "identifier": "ROBLOX_INTERNAL", + "env": "ROBLOX_INTERNAL" + }, "compute_expression", "remove_unused_if_branch", "remove_unused_while", @@ -22,4 +27,4 @@ "remove_nil_declaration", "remove_empty_do" ] -} \ No newline at end of file +} diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index 47ce996..1580f9f 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -37,6 +37,13 @@ local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: Mo result.name = module.Name:gsub(constants.STORYBOOK_NAME_PATTERN, "") end + -- selene: allow(global_usage) + if _G.ROBLOX_INTERNAL then + if result.group then + result.name = `{result.group} / {result.name}` + end + end + return result end From df83c1ac4306140b8092971e4b9ca6362b2ff82c Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 16:08:16 -0800 Subject: [PATCH 05/38] Mark roblox internal --- src/createRendererForStory.luau | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/createRendererForStory.luau b/src/createRendererForStory.luau index fed4a6d..05c7a74 100644 --- a/src/createRendererForStory.luau +++ b/src/createRendererForStory.luau @@ -23,8 +23,11 @@ type StoryRenderer = types.StoryRenderer local function createRendererForStory(story: Story): StoryRenderer local packages = if story.packages then story.packages else story.storybook.packages if packages then - if isReact(packages.Roact) or packages.React and packages.ReactRoblox then - return createRobloxInternalRenderer(packages) + -- selene: allow(global_usage) + if _G.ROBLOX_INTERNAL then + if isReact(packages.Roact) or packages.React and packages.ReactRoblox then + return createRobloxInternalRenderer(packages) + end end if packages.Roact then From 2aeaf782c7221ebdd583defc9fd0ecf593197496 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 16:16:26 -0800 Subject: [PATCH 06/38] Existing renderers work with Roblox stories --- src/createRendererForStory.luau | 9 -- src/migrateLegacyPackages.luau | 5 +- .../createRobloxInternalRenderer.luau | 60 ---------- .../createRobloxInternalRenderer.spec.luau | 107 ------------------ 4 files changed, 3 insertions(+), 178 deletions(-) delete mode 100644 src/renderers/createRobloxInternalRenderer.luau delete mode 100644 src/renderers/createRobloxInternalRenderer.spec.luau diff --git a/src/createRendererForStory.luau b/src/createRendererForStory.luau index 05c7a74..fcaf9c1 100644 --- a/src/createRendererForStory.luau +++ b/src/createRendererForStory.luau @@ -2,8 +2,6 @@ local createFusionRenderer = require("@root/renderers/createFusionRenderer") local createManualRenderer = require("@root/renderers/createManualRenderer") local createReactRenderer = require("@root/renderers/createReactRenderer") local createRoactRenderer = require("@root/renderers/createRoactRenderer") -local createRobloxInternalRenderer = require("@root/renderers/createRobloxInternalRenderer") -local isReact = require("@root/roblox-internal/isReact") local types = require("@root/types") type Story = types.Story @@ -23,13 +21,6 @@ type StoryRenderer = types.StoryRenderer local function createRendererForStory(story: Story): StoryRenderer local packages = if story.packages then story.packages else story.storybook.packages if packages then - -- selene: allow(global_usage) - if _G.ROBLOX_INTERNAL then - if isReact(packages.Roact) or packages.React and packages.ReactRoblox then - return createRobloxInternalRenderer(packages) - end - end - if packages.Roact then return createRoactRenderer(packages) elseif packages.React and packages.ReactRoblox then diff --git a/src/migrateLegacyPackages.luau b/src/migrateLegacyPackages.luau index 5b22843..a48364b 100644 --- a/src/migrateLegacyPackages.luau +++ b/src/migrateLegacyPackages.luau @@ -7,8 +7,9 @@ local function migrateLegacyPackages(storyOrStorybook: { [string]: any }): Story if storyOrStorybook.roact or storyOrStorybook.react or storyOrStorybook.reactRoblox then return { Roact = storyOrStorybook.roact, - RoactCompat = if isReact(storyOrStorybook.roact) then storyOrStorybook.roact else nil, - React = storyOrStorybook.react, + React = if storyOrStorybook.react + then storyOrStorybook.react + else if isReact(storyOrStorybook.roact) then storyOrStorybook.roact else nil, ReactRoblox = storyOrStorybook.reactRoblox, } end diff --git a/src/renderers/createRobloxInternalRenderer.luau b/src/renderers/createRobloxInternalRenderer.luau deleted file mode 100644 index bc2e848..0000000 --- a/src/renderers/createRobloxInternalRenderer.luau +++ /dev/null @@ -1,60 +0,0 @@ -local types = require("@root/types") - -type StoryRenderer = types.StoryRenderer - -type Packages = { - React: any, - RoactCompat: any, - ReactRoblox: any, -} - -local function isReactElement(maybeElement: any): boolean - if typeof(maybeElement) == "table" then - return maybeElement["$$typeof"] and maybeElement.props and maybeElement.type - end - return false -end - -local function createRobloxInternalRenderer(packages: Packages): StoryRenderer - local React = packages.React - local RoactCompat = packages.RoactCompat - local ReactRoblox = packages.ReactRoblox - - local root - local currentStory - - local function reactRender(story: types.Story, props) - local element - if isReactElement(story.story) then - element = story.story - else - element = React.createElement(story.story, props) - end - - currentStory = story - root:render(element) - end - - local function mount(container, story, props) - root = ReactRoblox.createRoot(container) - reactRender(story, props) - end - - local function update(props) - if currentStory then - reactRender(currentStory, props) - end - end - - local function unmount() - root:unmount() - end - - return { - mount = mount, - update = update, - unmount = unmount, - } -end - -return createRobloxInternalRenderer diff --git a/src/renderers/createRobloxInternalRenderer.spec.luau b/src/renderers/createRobloxInternalRenderer.spec.luau deleted file mode 100644 index 1390b80..0000000 --- a/src/renderers/createRobloxInternalRenderer.spec.luau +++ /dev/null @@ -1,107 +0,0 @@ -local Fusion = require("@pkg/Fusion") -local JestGlobals = require("@pkg/JestGlobals") - -local createStory = require("@root/test-utils/createStory") -local render = require("@root/render") - -local beforeEach = JestGlobals.beforeEach -local expect = JestGlobals.expect -local test = JestGlobals.test - -local New = Fusion.New -local Computed = Fusion.Computed -type StateObject = Fusion.StateObject - -local function TextStory() - return New("TextLabel")({ - Text = "Hello World", - }) -end - -local function ButtonStory(props: { - controls: { - isDisabled: StateObject, - }, -}) - return New("TextButton")({ - Text = Computed(function() - return if props.controls.isDisabled:get() then "Disabled" else "Enabled" - end), - }) -end - -local function createFusionStory(element) - return createStory(element, { - Fusion = Fusion, - }) -end - -local container - -beforeEach(function() - container = Instance.new("Folder") -end) - -test("render a Fusion component", function() - local story = createFusionStory(TextStory) - - render(container, story) - - expect(#container:GetChildren()).toBe(1) - expect(container:FindFirstChildWhichIsA("TextLabel")).toBeDefined() -end) - -test("unmount a Fusion component", function() - local story = createStory(TextStory) - local lifecycle = render(container, story) - - expect(#container:GetChildren()).toBe(1) - - lifecycle.unmount() - - expect(#container:GetChildren()).toBe(0) -end) - -test("controls are transformed into Values", function() - local story = createFusionStory(ButtonStory) - story.controls = { - isDisabled = true, - } - - render(container, story) - - local element = container:FindFirstChildWhichIsA("TextButton") - assert(element, "no element found") - expect(element.Text).toBe("Disabled") -end) - -test("update the component on arg changes", function() - expect(#container:GetChildren()).toBe(0) - - local story = createFusionStory(ButtonStory) - story.controls = { - isDisabled = true, - } - - local lifecycle = render(container, story) - - expect(#container:GetChildren()).toBe(1) - - local element = container:FindFirstChildWhichIsA("TextButton") - assert(element, "no element found") - expect(element.Text).toBe("Disabled") - - lifecycle.update({ - isDisabled = false, - }) - - expect(#container:GetChildren()).toBe(1) - - local prevElement = element - element = container:FindFirstChildWhichIsA("TextButton") - expect(element).toBe(prevElement) - - task.wait() - - expect(element.Text).toBe("Enabled") -end) From 404781af3d2c1f54b526199b8007b2a3d7dd2d43 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 16:25:03 -0800 Subject: [PATCH 07/38] Add mapStory support --- src/renderers/createReactRenderer.luau | 6 ++++++ src/renderers/createRoactRenderer.luau | 5 +++++ src/types.luau | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/renderers/createReactRenderer.luau b/src/renderers/createReactRenderer.luau index 24c7020..1062b1b 100644 --- a/src/renderers/createReactRenderer.luau +++ b/src/renderers/createReactRenderer.luau @@ -22,6 +22,8 @@ local function createReactRenderer(packages: Packages): StoryRenderer local currentStory local function reactRender(story: types.Story, props) + local mapStory = if story.storybook then story.storybook.mapStory else nil + local element if isReactElement(story.story) then element = story.story @@ -29,6 +31,10 @@ local function createReactRenderer(packages: Packages): StoryRenderer element = React.createElement(story.story, props) end + if mapStory then + element = React.createElement(mapStory(story), props, element) + end + currentStory = story root:render(element) end diff --git a/src/renderers/createRoactRenderer.luau b/src/renderers/createRoactRenderer.luau index 709f916..a942cc1 100644 --- a/src/renderers/createRoactRenderer.luau +++ b/src/renderers/createRoactRenderer.luau @@ -29,6 +29,11 @@ local function createRoactRenderer(packages: Packages): StoryRenderer element = Roact.createElement(currentComponent, props) end + local mapStory = if story.storybook then story.storybook.mapStory else nil + if mapStory then + element = Roact.createElement(mapStory(story.story), props, element) + end + tree = Roact.mount(element, container, "RoactRenderer") end diff --git a/src/types.luau b/src/types.luau index ed81816..6e2a7ff 100644 --- a/src/types.luau +++ b/src/types.luau @@ -49,7 +49,7 @@ export type Storybook = { name: string?, packages: StoryPackages?, - -- mapStory: ((story: Story) -> Story)?, + mapStory: ((story: any) -> (props: StoryProps) -> any)?, } types.IStorybook = t.interface({ storyRoots = t.array(t.Instance), From c66fb444fcf73d0d5794ce4080eedc5b402f366a Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 16:35:51 -0800 Subject: [PATCH 08/38] Choose any random substory to render as the story --- src/loadStoryModule.luau | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 0a0f032..1f25016 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -51,6 +51,18 @@ local function loadStoryModule( } end + -- selene: allow(global_usage) + if _G.ROBLOX_INTERNAL then + if result.stories and not result.story then + local randomSubstory + for _, substory in result.stories do + randomSubstory = substory + break + end + result.story = randomSubstory.story + end + end + local isValid, message = types.IStory(result) if not isValid then From 6c574baa2460996e06aaf75073a8a0fd15e764f8 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Thu, 7 Nov 2024 16:50:36 -0800 Subject: [PATCH 09/38] Use Prospector for immediate performance improvement --- src/matchDescendants.luau | 18 +++++++----------- wally.toml | 1 + 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/matchDescendants.luau b/src/matchDescendants.luau index 0f114af..ef8682b 100644 --- a/src/matchDescendants.luau +++ b/src/matchDescendants.luau @@ -1,3 +1,5 @@ +local Prospector = require("@pkg/Prospector") + local function hasPermission(instance: Instance) local success = pcall(function() return instance.Name @@ -12,17 +14,11 @@ local function matchDescendants(parent: Instance, predicate: (descendant: Instan return matches end - for _, descendant in parent:GetDescendants() do - local success, result = pcall(function() - return predicate(descendant) - end) - - if success and result == true then - table.insert(matches, descendant) - end - end - - return matches + -- Before Prospector we were calling parent:GetDescendants() but because we + -- need to support the DataModel being passed in that case would allocate a + -- gigantic array. Prospector will traverse the hierarchy manually, which + -- takes longer but won't freeze up consumers + return Prospector.Collect.descendantsDFS(parent, predicate) end return matchDescendants diff --git a/wally.toml b/wally.toml index d0a0406..261430b 100644 --- a/wally.toml +++ b/wally.toml @@ -19,6 +19,7 @@ Sift = "csqrl/sift@0.0.8" ModuleLoader = "flipbook-labs/module-loader@0.6.2" t = "osyrisrblx/t@3.0.0" React = "jsdotlua/react@17.0.2" +Prospector = "egomoose/prospector@1.1.0" # [dev-dependencies] ReactRoblox = "jsdotlua/react-roblox@17.0.2" From a85500e330ed75fa06b5281093804d4569edd52e Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Thu, 21 Nov 2024 15:40:10 -0800 Subject: [PATCH 10/38] Add a todo --- src/loadStoryModule.luau | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 999f223..85a926e 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -55,6 +55,9 @@ local function loadStoryModule( -- selene: allow(global_usage) if _G.ROBLOX_INTERNAL then + -- TODO: We need support for substories to be compatible with Developer + -- Storybook. In the meantime we're just selecting a randomly accessed + -- one if result.stories and not result.story then local randomSubstory for _, substory in result.stories do From bac365869d38b8eff7bf03cb7f9ff53d0329df86 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 11 Dec 2024 20:32:03 -0800 Subject: [PATCH 11/38] Don't bother gating behind a flag --- .darklua.json | 7 +------ src/loadStoryModule.luau | 21 +++++++++------------ src/loadStorybookModule.luau | 7 ++----- 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/.darklua.json b/.darklua.json index da21440..183d337 100644 --- a/.darklua.json +++ b/.darklua.json @@ -15,11 +15,6 @@ "indexing_style": "property" } }, - { - "rule": "inject_global_value", - "identifier": "ROBLOX_INTERNAL", - "env": "ROBLOX_INTERNAL" - }, "compute_expression", "remove_unused_if_branch", "remove_unused_while", @@ -27,4 +22,4 @@ "remove_nil_declaration", "remove_empty_do" ] -} +} \ No newline at end of file diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 85a926e..a4645b4 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -53,19 +53,16 @@ local function loadStoryModule( } end - -- selene: allow(global_usage) - if _G.ROBLOX_INTERNAL then - -- TODO: We need support for substories to be compatible with Developer - -- Storybook. In the meantime we're just selecting a randomly accessed - -- one - if result.stories and not result.story then - local randomSubstory - for _, substory in result.stories do - randomSubstory = substory - break - end - result.story = randomSubstory.story + -- TODO: We need support for substories to be compatible with Developer + -- Storybook. In the meantime we're just selecting a randomly accessed + -- one + if result.stories and not result.story then + local randomSubstory + for _, substory in result.stories do + randomSubstory = substory + break end + result.story = randomSubstory.story end local isValid, message = types.IStory(result) diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index 3148fce..a873a64 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -42,11 +42,8 @@ local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: Mo end end - -- selene: allow(global_usage) - if _G.ROBLOX_INTERNAL then - if result.group then - result.name = `{result.group} / {result.name}` - end + if result.group then + result.name = `{result.group} / {result.name}` end return result From d075fc1f77ee0bc534ec64ef3b70e46d448ff24f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 10:07:14 -0800 Subject: [PATCH 12/38] Handle more substory cases --- src/loadStoryModule.luau | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index a4645b4..060b9e2 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -62,7 +62,11 @@ local function loadStoryModule( randomSubstory = substory break end - result.story = randomSubstory.story + if typeof(randomSubstory) == "table" then + result.story = randomSubstory.story + else + result.story = randomSubstory + end end local isValid, message = types.IStory(result) From d22446ec6f5f7ff554e7e44139c9d7da21b75d6d Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 11:17:43 -0800 Subject: [PATCH 13/38] Adjust internal handling for React/ReactRoblox --- src/migrateLegacyPackages.luau | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/migrateLegacyPackages.luau b/src/migrateLegacyPackages.luau index a48364b..348229a 100644 --- a/src/migrateLegacyPackages.luau +++ b/src/migrateLegacyPackages.luau @@ -5,11 +5,19 @@ type StoryPackages = types.StoryPackages local function migrateLegacyPackages(storyOrStorybook: { [string]: any }): StoryPackages? if storyOrStorybook.roact or storyOrStorybook.react or storyOrStorybook.reactRoblox then + print("storyOrStorybook", storyOrStorybook) + + -- Developer Storybook + if storyOrStorybook.roact and storyOrStorybook.reactRoblox and isReact(storyOrStorybook.roact) then + return { + React = storyOrStorybook.roact, + ReactRoblox = storyOrStorybook.reactRoblox, + } + end + return { Roact = storyOrStorybook.roact, - React = if storyOrStorybook.react - then storyOrStorybook.react - else if isReact(storyOrStorybook.roact) then storyOrStorybook.roact else nil, + React = storyOrStorybook.react, ReactRoblox = storyOrStorybook.reactRoblox, } end From d58594c9b43ffac5e45415ed7be39fd710602ec9 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 11:28:39 -0800 Subject: [PATCH 14/38] Fix mapStory for React storybooks --- src/renderers/createReactRenderer.luau | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/renderers/createReactRenderer.luau b/src/renderers/createReactRenderer.luau index bdded3e..d7f38e0 100644 --- a/src/renderers/createReactRenderer.luau +++ b/src/renderers/createReactRenderer.luau @@ -22,8 +22,6 @@ local function createReactRenderer(packages: Packages): StoryRenderer local currentStory local function reactRender(story: types.LoadedStory, props) - local mapStory = if story.storybook then story.storybook.mapStory else nil - local element if isReactElement(story.story) then element = story.story @@ -31,8 +29,9 @@ local function createReactRenderer(packages: Packages): StoryRenderer element = React.createElement(story.story, props) end + local mapStory = if story.storybook then story.storybook.mapStory else nil if mapStory then - element = React.createElement(mapStory(story), props, element) + element = React.createElement(mapStory(story.story), props, element) end currentStory = story From 6be8b08ae31015e6f73380b1ac15381361b0b893 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 12:58:00 -0800 Subject: [PATCH 15/38] Fix build watching --- .lune/build.luau | 1 - 1 file changed, 1 deletion(-) diff --git a/.lune/build.luau b/.lune/build.luau index 2b40061..7353339 100644 --- a/.lune/build.luau +++ b/.lune/build.luau @@ -48,7 +48,6 @@ if args.watch then watch({ filePatterns = { "src/.*%.luau", - "example/.*%.luau", }, onChanged = build, }) From e0b1716e3e5c0179b5ba16de2e9421f1d1342bc0 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 13:52:44 -0800 Subject: [PATCH 16/38] Remove debug print --- src/migrateLegacyPackages.luau | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/migrateLegacyPackages.luau b/src/migrateLegacyPackages.luau index 348229a..9e9a5c5 100644 --- a/src/migrateLegacyPackages.luau +++ b/src/migrateLegacyPackages.luau @@ -5,8 +5,6 @@ type StoryPackages = types.StoryPackages local function migrateLegacyPackages(storyOrStorybook: { [string]: any }): StoryPackages? if storyOrStorybook.roact or storyOrStorybook.react or storyOrStorybook.reactRoblox then - print("storyOrStorybook", storyOrStorybook) - -- Developer Storybook if storyOrStorybook.roact and storyOrStorybook.reactRoblox and isReact(storyOrStorybook.roact) then return { From 08a547dc8c7918e7c94f5d823c4766529193e42b Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 14:55:19 -0800 Subject: [PATCH 17/38] Expose UnavailableStorybook type --- src/hooks/useStorybooks.luau | 2 +- src/init.luau | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 9acc24f..f9e8678 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -6,7 +6,7 @@ local isStorybookModule = require("@root/isStorybookModule") local loadStorybookModule = require("@root/loadStorybookModule") local types = require("@root/types") -type UnavailableStorybook = { +export type UnavailableStorybook = { problem: string, storybook: types.LoadedStorybook, } diff --git a/src/init.luau b/src/init.luau index 2bb0cdc..acc1851 100644 --- a/src/init.luau +++ b/src/init.luau @@ -15,6 +15,8 @@ local types = require("./types") @class Storyteller ]=] +local useStorybooks = require("./hooks/useStorybooks") + export type RenderLifecycle = types.RenderLifecycle export type Story = types.Story export type LoadedStory = types.LoadedStory @@ -23,6 +25,7 @@ export type LoadedStorybook = types.LoadedStorybook export type StoryControls = types.StoryControls export type StoryProps = types.StoryProps export type StoryRenderer = types.StoryRenderer +export type UnavailableStorybook = useStorybooks.UnavailableStorybook return { -- Validation @@ -42,5 +45,5 @@ return { -- Hooks useStory = require("./hooks/useStory"), - useStorybooks = require("./hooks/useStorybooks"), + useStorybooks = useStorybooks, } From a036d9ac83f39af8c0d33ecf4ef44e4905c9a993 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 09:48:01 -0800 Subject: [PATCH 18/38] Fix mapStory for Roact renderer --- src/renderers/createRoactRenderer.luau | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/renderers/createRoactRenderer.luau b/src/renderers/createRoactRenderer.luau index a942cc1..08e0f55 100644 --- a/src/renderers/createRoactRenderer.luau +++ b/src/renderers/createRoactRenderer.luau @@ -1,6 +1,8 @@ local types = require("@root/types") type StoryRenderer = types.StoryRenderer +type LoadedStory = types.LoadedStory +type StoryProps = types.StoryProps type Packages = { Roact: any, @@ -13,16 +15,17 @@ local function isRoactElement(maybeElement: any): boolean return false end -local function createRoactRenderer(packages: Packages): StoryRenderer +local function createRoactRenderer(packages: Packages): StoryRenderer local Roact = packages.Roact local tree local currentComponent + local currentStory - local function mount(container, story, props) + local function mount(container: Instance, story: LoadedStory, props: StoryProps) local element if isRoactElement(story.story) then - currentComponent = story.story.component + currentComponent = (story.story :: any).component element = story.story else currentComponent = story.story @@ -34,12 +37,19 @@ local function createRoactRenderer(packages: Packages): StoryRenderer element = Roact.createElement(mapStory(story.story), props, element) end + currentStory = story tree = Roact.mount(element, container, "RoactRenderer") end - local function update(props) + local function update(props: StoryProps) if tree and currentComponent then local element = Roact.createElement(currentComponent, props) + + local mapStory = if currentStory and currentStory.storybook then currentStory.storybook.mapStory else nil + if mapStory then + element = Roact.createElement(mapStory(currentStory.story), props, element) + end + Roact.update(tree, element) end end From 3611a6515d48aa34714d3a8909f9dfd279a4ad97 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 09:36:13 -0800 Subject: [PATCH 19/38] Each Storybook has its own ModuleLoader for better sandboxing 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 --- src/e2e/e2e.spec.luau | 8 +- src/findStoryModulesForStorybook.spec.luau | 2 + src/hooks/useStory.luau | 39 ++++---- src/hooks/useStory.spec.luau | 14 ++- src/hooks/useStorybooks.luau | 44 ++++++--- src/hooks/useStorybooks.spec.luau | 14 +-- src/loadStoryModule.luau | 9 +- src/loadStoryModule.spec.luau | 103 +++++++++++---------- src/loadStorybookModule.luau | 9 +- src/loadStorybookModule.spec.luau | 12 +-- src/render.luau | 5 +- src/test-utils/createStory.luau | 4 + src/types.luau | 6 +- 13 files changed, 148 insertions(+), 121 deletions(-) diff --git a/src/e2e/e2e.spec.luau b/src/e2e/e2e.spec.luau index c2d2797..4ad7b11 100644 --- 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 index a3c384d..28c6a85 100644 --- 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,7 @@ test("find all story modules for a storybook", function() local storybook: types.LoadedStorybook = { name = "Storybook", + loader = ModuleLoader.new(), storyRoots = { container, }, diff --git a/src/hooks/useStory.luau b/src/hooks/useStory.luau index eb20bd7..1c7fdb1 100644 --- 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,32 +52,27 @@ local types = require("@root/types") @tag React @tag Story @within Storyteller - ]=] -local function useStory( - module: ModuleScript, - storybook: types.LoadedStorybook, - loader: ModuleLoader.ModuleLoader -): (types.LoadedStory?, string?) +local function useStory(module: ModuleScript, storybook: types.LoadedStorybook): (types.LoadedStory?, string?) local state, setState = React.useState({} :: { story: types.LoadedStory?, err: string?, }) - local loadStory = React.useCallback(function() - local story - local success, err = pcall(function() - story = loadStoryModule(loader, module, storybook) - end) - - setState({ - story = if success then story else nil, - err = if not success then err else nil, - }) - end, { loader, module, storybook } :: { unknown }) - React.useEffect(function() - local conn = loader.loadedModuleChanged:Connect(function(other) + local function loadStory() + local story + local success, err = pcall(function() + story = loadStoryModule(module, storybook) + end) + + setState({ + story = if success then story else nil, + err = if not success then err else nil, + }) + end + + local conn = storybook.loader.loadedModuleChanged:Connect(function(other) if other == module then loadStory() end @@ -90,7 +83,7 @@ local function useStory( return function() conn:Disconnect() end - end, { module, loadStory, loader } :: { unknown }) + end, { module, storybook } :: { unknown }) return state.story, state.err end diff --git a/src/hooks/useStory.spec.luau b/src/hooks/useStory.spec.luau index f9a4a51..152b7eb 100644 --- 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,14 @@ beforeEach(function() storybook = { name = "Storybook", + loader = ModuleLoader.new(), 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 +75,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,7 +86,7 @@ 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() @@ -118,7 +116,7 @@ end) test("reloads on dependency changes", function() local get = renderHook(function() - return useStory(storyModule, storybook, loader) + return useStory(storyModule, storybook) end) local story = get() diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index f9e8678..1c391d9 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -6,6 +6,10 @@ local isStorybookModule = require("@root/isStorybookModule") local loadStorybookModule = require("@root/loadStorybookModule") local types = require("@root/types") +local useRef = React.useRef +local useState = React.useState +local useCallback = React.useCallback + export type UnavailableStorybook = { problem: string, storybook: types.LoadedStorybook, @@ -66,24 +70,38 @@ export type UnavailableStorybook = { @tag Storybook @within Storyteller ]=] -local function useStorybooks( - parent: Instance, - loader: ModuleLoader.ModuleLoader -): { +local function useStorybooks(parent: Instance): { available: { types.LoadedStorybook }, unavailable: { UnavailableStorybook }, } - local storybooks, setStorybooks = React.useState({}) - local unavailableStorybooks, setUnavailableStorybooks = React.useState({} :: { UnavailableStorybook }) + local storybooks, setStorybooks = useState({}) + local unavailableStorybooks, setUnavailableStorybooks = useState({} :: { UnavailableStorybook }) + + local loaders = useRef({} :: { [ModuleScript]: ModuleLoader.ModuleLoader }) + + local getOrCreateLoader = useCallback(function(storybookModule: ModuleScript) + local existingLoader = loaders.current[storybookModule] + if existingLoader then + return existingLoader + else + local loader = ModuleLoader.new() + loaders.current[storybookModule] = loader + return loader + end + end, {}) - local loadStorybooks = React.useCallback(function() + local loadStorybooks = useCallback(function() local newStorybooks = {} local newUnavailableStorybooks: { UnavailableStorybook } = {} for _, storybookModule in findStorybookModules(parent) do + local loader = getOrCreateLoader(storybookModule) + + -- TODO: Do we need to clear the loader first? + local storybook: types.LoadedStorybook? local success, result = pcall(function() - storybook = loadStorybookModule(loader, storybookModule) + storybook = loadStorybookModule(storybookModule, loader) end) if success then @@ -93,6 +111,7 @@ local function useStorybooks( problem = result, storybook = { name = storybookModule.Name, + loader = loader, storyRoots = {}, }, }) @@ -101,7 +120,7 @@ local function useStorybooks( setStorybooks(newStorybooks) setUnavailableStorybooks(newUnavailableStorybooks) - end, { parent, loader } :: { unknown }) + end, { parent, getOrCreateLoader } :: { unknown }) React.useEffect(function() local function reloadIfStorybook(instance: Instance) @@ -111,10 +130,13 @@ local function useStorybooks( end local connections = { - loader.loadedModuleChanged:Connect(reloadIfStorybook), parent.DescendantAdded:Connect(reloadIfStorybook), } + for _, loader in loaders.current do + table.insert(connections, loader.loadedModuleChanged:Connect(reloadIfStorybook)) + end + loadStorybooks() return function() @@ -122,7 +144,7 @@ local function useStorybooks( conn:Disconnect() end end - end, { loadStorybooks, loader, parent } :: { unknown }) + end, { loadStorybooks, parent } :: { unknown }) return { available = storybooks, diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index bce574b..43ef7b3 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -1,5 +1,4 @@ local JestGlobals = require("@pkg/JestGlobals") -local ModuleLoader = require("@pkg/ModuleLoader") local ReactRoblox = require("@pkg/ReactRoblox") local renderHook = require("@root/test-utils/renderHook") @@ -14,13 +13,10 @@ local act = ReactRoblox.act type LoadedStorybook = types.LoadedStorybook -local loader: ModuleLoader.ModuleLoader local container: Folder local storybookModule: ModuleScript beforeEach(function() - loader = ModuleLoader.new() - container = Instance.new("Folder") storybookModule = Instance.new("ModuleScript") @@ -37,7 +33,7 @@ end) test("loads storybook modules", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() @@ -58,7 +54,7 @@ test("storybooks with syntax errors are marked unavailable", function() ]] local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() @@ -86,7 +82,7 @@ test("adding a new storybook to the DM triggers a re-render", function() ]] local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() @@ -119,7 +115,7 @@ end) -- TOOD: Update useStorybooks so this test can pass test.skip("removing a storybook from the DM triggers a re-render", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) act(function() @@ -133,7 +129,7 @@ end) test("re-renders on module changes", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 060b9e2..28a1db5 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -1,4 +1,3 @@ -local ModuleLoader = require("@pkg/ModuleLoader") local Sift = require("@pkg/Sift") local constants = require("@root/constants") @@ -28,13 +27,9 @@ type LoadedStory = types.LoadedStory @tag Module Loading @within Storyteller ]=] -local function loadStoryModule( - loader: ModuleLoader.ModuleLoader, - storyModule: ModuleScript, - storybook: LoadedStorybook -): LoadedStory +local function loadStoryModule(storyModule: ModuleScript, storybook: LoadedStorybook): LoadedStory local success, result = pcall(function() - return loader:require(storyModule) + return storybook.loader:require(storyModule) end) if not success then diff --git a/src/loadStoryModule.spec.luau b/src/loadStoryModule.spec.luau index fc02414..f7e4c2a 100644 --- a/src/loadStoryModule.spec.luau +++ b/src/loadStoryModule.spec.luau @@ -8,36 +8,51 @@ local describe = JestGlobals.describe local jest = JestGlobals.jest local test = JestGlobals.test local expect = JestGlobals.expect - -local MOCK_PLAIN_STORYBOOK: types.LoadedStorybook = { - name = "Plain Storybook", - storyRoots = {}, -} - -local MOCK_ROACT_STORYBOOK: types.LoadedStorybook = { - name = "Roact Storybook", - storyRoots = {}, - packages = { - Roact = { - createElement = jest.fn(), - mount = jest.fn(), - unmount = jest.fn(), - }, - }, -} - -local MOCK_REACT_STORYBOOK: types.LoadedStorybook = { - name = "React Storybook", - storyRoots = {}, - packages = { - React = { - createElement = jest.fn(), +local beforeEach = JestGlobals.beforeEach +local afterEach = JestGlobals.afterEach + +local mockPlainStorybook: types.LoadedStorybook +local mockRoactStorybook: types.LoadedStorybook +local mockReactStorybook: types.LoadedStorybook + +beforeEach(function() + mockPlainStorybook = { + name = "Plain Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + } + + mockRoactStorybook = { + name = "Roact Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + packages = { + Roact = { + createElement = jest.fn(), + mount = jest.fn(), + unmount = jest.fn(), + }, }, - ReactRoblox = { - createRoot = jest.fn(), + } + + mockReactStorybook = { + name = "React Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + packages = { + React = { + createElement = jest.fn(), + }, + ReactRoblox = { + createRoot = jest.fn(), + }, }, - }, -} + } +end) + +afterEach(function() + jest.resetAllMocks() +end) local function createMockStoryModule(source: string): ModuleScript local storyModule = Instance.new("ModuleScript") @@ -48,7 +63,6 @@ local function createMockStoryModule(source: string): ModuleScript end test("load a story module as a table", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { name = "Sample", @@ -56,18 +70,17 @@ test("load a story module as a table", function() } ]]) - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) expect(story).toEqual({ name = "Sample", source = expect.anything(), story = expect.any("function"), - storybook = MOCK_PLAIN_STORYBOOK, + storybook = mockPlainStorybook, }) end) test("handle Hoarcekat stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return function(target) local gui = Instance.new("TextLabel") @@ -79,13 +92,12 @@ test("handle Hoarcekat stories", function() end ]]) - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) expect(story).toBeDefined() end) test("use the name of the story module for the story name", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { story = function() end @@ -93,31 +105,29 @@ test("use the name of the story module for the story name", function() ]]) storyModule.Name = "SampleName.story" - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) assert(story ~= nil, "story not defined") expect(story.name).toEqual("SampleName") end) test("pass the storybook's renderer to the story", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { story = function() end } ]]) - local story = loadStoryModule(loader, storyModule, MOCK_REACT_STORYBOOK) + local story = loadStoryModule(storyModule, mockReactStorybook) expect(story).toBeDefined() - story = loadStoryModule(loader, storyModule, MOCK_ROACT_STORYBOOK) + story = loadStoryModule(storyModule, mockRoactStorybook) expect(story).toBeDefined() end) test("generic failures for stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ syntax error return { @@ -126,12 +136,11 @@ test("generic failures for stories", function() ]]) expect(function() - loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + loadStoryModule(storyModule, mockPlainStorybook) end).toThrow("Failed to load story") end) test("malformed stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { name = false, @@ -140,17 +149,16 @@ test("malformed stories", function() ]]) expect(function() - loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + loadStoryModule(storyModule, mockPlainStorybook) end).toThrow("Story is malformed") end) describe("legacy support", function() test("packages attached to the story get grouped in `packages` object", function() - local loader = ModuleLoader.new() - local storybook: types.LoadedStorybook = { name = "Storybook", storyRoots = {}, + loader = ModuleLoader.new(), } local storyModule = createMockStoryModule([[ @@ -161,7 +169,7 @@ describe("legacy support", function() } ]]) - local story = loadStoryModule(loader, storyModule, storybook) + local story = loadStoryModule(storyModule, storybook) expect(story).toMatchObject({ story = expect.anything(), @@ -173,11 +181,10 @@ describe("legacy support", function() end) test("packages attached to the story take precedence over the storybook", function() - local loader = ModuleLoader.new() - local storybook: types.LoadedStorybook = { name = "Storybook", storyRoots = {}, + loader = ModuleLoader.new(), packages = { Roact = {}, }, @@ -191,7 +198,7 @@ describe("legacy support", function() } ]]) - local story = loadStoryModule(loader, storyModule, storybook) + local story = loadStoryModule(storyModule, storybook) expect(story).toMatchObject({ story = expect.anything(), diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index a873a64..fe12805 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -4,6 +4,9 @@ local constants = require("@root/constants") local migrateLegacyPackages = require("@root/migrateLegacyPackages") local types = require("@root/types") +type LoadedStorybook = types.LoadedStorybook +type ModuleLoader = ModuleLoader.ModuleLoader + --[=[ Loads the source of a Storybook module. @@ -18,7 +21,9 @@ local types = require("@root/types") @tag Module Loading @within Storyteller ]=] -local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: ModuleScript): types.LoadedStorybook +local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleLoader?): LoadedStorybook + local loader = if providedLoader then providedLoader else ModuleLoader.new() + local wasRequired, result = pcall(function() return loader:require(module) end) @@ -46,6 +51,8 @@ local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: Mo result.name = `{result.group} / {result.name}` end + result.loader = loader + return result end diff --git a/src/loadStorybookModule.spec.luau b/src/loadStorybookModule.spec.luau index 52a8f75..3b196f8 100644 --- a/src/loadStorybookModule.spec.luau +++ b/src/loadStorybookModule.spec.luau @@ -26,7 +26,7 @@ test("load a storybook module as a table", function() } ]]) - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("Sample") expect(#storybook.storyRoots).toBe(1) @@ -41,7 +41,7 @@ test("use the name of the storybook module by default", function() ]]) storybookModule.Name = "SampleStorybook.storybook" - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("SampleStorybook") end) @@ -55,7 +55,7 @@ test("nameless storybook", function() ]]) storybookModule.Name = ".storybook" - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("Unnamed Storybook") end) @@ -68,7 +68,7 @@ test("generic failures for storybook", function() ]]) local success, result = pcall(function() - return loadStorybookModule(loader, storybookModule) + return loadStorybookModule(storybookModule, loader) end) expect(success).toBeFalsy() @@ -85,7 +85,7 @@ test("malformed storybook", function() ]]) local success, result = pcall(function() - return loadStorybookModule(loader, storybookModule) + return loadStorybookModule(storybookModule, loader) end) expect(success).toBeFalsy() @@ -106,7 +106,7 @@ describe("legacy support", function() } ]]) - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook).toMatchObject({ storyRoots = expect.anything(), diff --git a/src/render.luau b/src/render.luau index 2136641..1fc56b0 100644 --- a/src/render.luau +++ b/src/render.luau @@ -37,7 +37,6 @@ end local ModuleLoader = require("@pkg/ModuleLoader") local Storyteller = require("@pkg/Storyteller") - local loader = ModuleLoader.new() -- At least one `.storybook` module must be present local storybookModules = Storyteller.findStorybookModules(game) @@ -45,7 +44,7 @@ end local storybook pcall(function() - storybook = Storyteller.loadStorybookModule(loader, storybookModules[1]) + storybook = Storyteller.loadStorybookModule(storybookModules[1]) end) if storybook then @@ -56,7 +55,7 @@ end local story pcall(function() - story = Storyteller.loadStoryModule(loader, storyModules[1], storybook) + story = Storyteller.loadStoryModule(storyModules[1], storybook) end) if story then diff --git a/src/test-utils/createStory.luau b/src/test-utils/createStory.luau index fdf6b36..b3dd9a1 100644 --- a/src/test-utils/createStory.luau +++ b/src/test-utils/createStory.luau @@ -1,3 +1,5 @@ +local ModuleLoader = require("@pkg/ModuleLoader") + local types = require("@root/types") local function createStory(element: T, packages: types.StoryPackages?): types.LoadedStory @@ -6,6 +8,8 @@ local function createStory(element: T, packages: types.StoryPackages?): types story = element, source = Instance.new("ModuleScript"), storybook = { + name = "Storybook", + loader = ModuleLoader.new(), storyRoots = {}, }, packages = packages, diff --git a/src/types.luau b/src/types.luau index 13fcab1..642e856 100644 --- a/src/types.luau +++ b/src/types.luau @@ -1,5 +1,8 @@ +local ModuleLoader = require("@pkg/ModuleLoader") local t = require("@pkg/t") +type ModuleLoader = ModuleLoader.ModuleLoader + local types = {} export type StudioTheme = "Light" | "Dark" @@ -60,6 +63,7 @@ types.IStorybook = t.interface({ export type LoadedStorybook = { name: string, storyRoots: { Instance }, + loader: ModuleLoader, packages: StoryPackages?, } @@ -84,7 +88,7 @@ export type LoadedStory = { name: string, story: T | (props: StoryProps) -> T, source: ModuleScript, - storybook: Storybook, + storybook: LoadedStorybook, packages: StoryPackages?, summary: string?, From 23e599b99740a29630c4a9be0ade3b7a24c4609a Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 10:40:21 -0800 Subject: [PATCH 20/38] Storybooks keep track of their source module --- src/findStoryModulesForStorybook.spec.luau | 1 + src/hooks/useStory.spec.luau | 1 + src/loadStoryModule.spec.luau | 5 +++++ src/loadStorybookModule.luau | 1 + src/test-utils/createStory.luau | 1 + src/types.luau | 1 + 6 files changed, 10 insertions(+) diff --git a/src/findStoryModulesForStorybook.spec.luau b/src/findStoryModulesForStorybook.spec.luau index 28c6a85..8de788f 100644 --- a/src/findStoryModulesForStorybook.spec.luau +++ b/src/findStoryModulesForStorybook.spec.luau @@ -29,6 +29,7 @@ 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.spec.luau b/src/hooks/useStory.spec.luau index 152b7eb..1c83b88 100644 --- a/src/hooks/useStory.spec.luau +++ b/src/hooks/useStory.spec.luau @@ -50,6 +50,7 @@ beforeEach(function() storybook = { name = "Storybook", loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), storyRoots = { container }, } end) diff --git a/src/loadStoryModule.spec.luau b/src/loadStoryModule.spec.luau index f7e4c2a..0369d01 100644 --- a/src/loadStoryModule.spec.luau +++ b/src/loadStoryModule.spec.luau @@ -20,12 +20,14 @@ beforeEach(function() name = "Plain Storybook", storyRoots = {}, loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), } mockRoactStorybook = { name = "Roact Storybook", storyRoots = {}, loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), packages = { Roact = { createElement = jest.fn(), @@ -39,6 +41,7 @@ beforeEach(function() name = "React Storybook", storyRoots = {}, loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), packages = { React = { createElement = jest.fn(), @@ -159,6 +162,7 @@ describe("legacy support", function() name = "Storybook", storyRoots = {}, loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), } local storyModule = createMockStoryModule([[ @@ -185,6 +189,7 @@ describe("legacy support", function() name = "Storybook", storyRoots = {}, loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), packages = { Roact = {}, }, diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index fe12805..f7e2478 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -51,6 +51,7 @@ local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleL result.name = `{result.group} / {result.name}` end + result.source = module result.loader = loader return result diff --git a/src/test-utils/createStory.luau b/src/test-utils/createStory.luau index b3dd9a1..00cc789 100644 --- a/src/test-utils/createStory.luau +++ b/src/test-utils/createStory.luau @@ -10,6 +10,7 @@ local function createStory(element: T, packages: types.StoryPackages?): types storybook = { name = "Storybook", loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), storyRoots = {}, }, packages = packages, diff --git a/src/types.luau b/src/types.luau index 642e856..93b6831 100644 --- a/src/types.luau +++ b/src/types.luau @@ -64,6 +64,7 @@ export type LoadedStorybook = { name: string, storyRoots: { Instance }, loader: ModuleLoader, + source: ModuleScript, packages: StoryPackages?, } From a4afdd0695d52a3259a4afab0a9347bd525a6ee1 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 10:42:10 -0800 Subject: [PATCH 21/38] Fix unit tests and type errors --- src/hooks/useStorybooks.luau | 134 +++++++++++++++----------- src/hooks/useStorybooks.spec.luau | 21 ++-- src/init.luau | 2 +- src/roblox-internal/isReact.spec.luau | 6 +- src/types.luau | 21 ++-- 5 files changed, 96 insertions(+), 88 deletions(-) diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 1c391d9..69484d1 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -1,19 +1,20 @@ local ModuleLoader = require("@pkg/ModuleLoader") local React = require("@pkg/React") +local Sift = require("@pkg/Sift") local findStorybookModules = require("@root/findStorybookModules") local isStorybookModule = require("@root/isStorybookModule") local loadStorybookModule = require("@root/loadStorybookModule") local types = require("@root/types") +local useCallback = React.useCallback +local useEffect = React.useEffect local useRef = React.useRef local useState = React.useState -local useCallback = React.useCallback -export type UnavailableStorybook = { - problem: string, - storybook: types.LoadedStorybook, -} +type ModuleLoader = ModuleLoader.ModuleLoader +type LoadedStorybook = types.LoadedStorybook +type UnavailableStorybook = types.UnavailableStorybook --[=[ Performs all the discovery and loading of Storybook modules that would @@ -36,9 +37,8 @@ export type UnavailableStorybook = { local function StorybookList(props: { parent: Instance, - loader: ModuleLoader, }) - local storybooks = Storyteller.useStorybooks(props.parent, props.loader) + local storybooks = Storyteller.useStorybooks(props.parent) local children = {} for index, storybook in storybooks do @@ -71,80 +71,100 @@ export type UnavailableStorybook = { @within Storyteller ]=] local function useStorybooks(parent: Instance): { - available: { types.LoadedStorybook }, + available: { LoadedStorybook }, unavailable: { UnavailableStorybook }, } - local storybooks, setStorybooks = useState({}) + local connections = useRef({} :: { RBXScriptConnection }) + local storybooks, setStorybooks = useState({} :: { LoadedStorybook }) local unavailableStorybooks, setUnavailableStorybooks = useState({} :: { UnavailableStorybook }) - local loaders = useRef({} :: { [ModuleScript]: ModuleLoader.ModuleLoader }) - - local getOrCreateLoader = useCallback(function(storybookModule: ModuleScript) - local existingLoader = loaders.current[storybookModule] - if existingLoader then - return existingLoader + local loadStorybook = useCallback(function(storybookModule: ModuleScript, loader: ModuleLoader) + local storybook: LoadedStorybook? + local success, result = pcall(function() + storybook = loadStorybookModule(storybookModule, loader) + end) + + -- TODO: The logic to add/update storybooks at their index is nasty. + -- Revise this before merging + if success and storybook then + setStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other) + return other.source == storybookModule + end) + + if index then + new[index] = storybook + else + table.insert(new, storybook) + end + + return new + end) else - local loader = ModuleLoader.new() - loaders.current[storybookModule] = loader - return loader + local unavailableStorybook: UnavailableStorybook = { + problem = result, + storybook = { + name = storybookModule.Name, + loader = loader, + source = storybookModule, + storyRoots = {}, + }, + } + + setUnavailableStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other) + return other.storybook.source == storybookModule + end) + + if index then + new[index] = unavailableStorybook + else + table.insert(new, unavailableStorybook) + end + + return new + end) end end, {}) - local loadStorybooks = useCallback(function() - local newStorybooks = {} - local newUnavailableStorybooks: { UnavailableStorybook } = {} + local loadAllStorybooks = useCallback(function() + setStorybooks({}) + setUnavailableStorybooks({}) for _, storybookModule in findStorybookModules(parent) do - local loader = getOrCreateLoader(storybookModule) - - -- TODO: Do we need to clear the loader first? + local loader = ModuleLoader.new() - local storybook: types.LoadedStorybook? - local success, result = pcall(function() - storybook = loadStorybookModule(storybookModule, loader) - end) + table.insert( + connections.current, + loader.loadedModuleChanged:Connect(function() + loader:clear() + loadStorybook(storybookModule, loader) + end) + ) - if success then - table.insert(newStorybooks, storybook) - else - table.insert(newUnavailableStorybooks, { - problem = result, - storybook = { - name = storybookModule.Name, - loader = loader, - storyRoots = {}, - }, - }) - end + loadStorybook(storybookModule, loader) end + end, { parent }) - setStorybooks(newStorybooks) - setUnavailableStorybooks(newUnavailableStorybooks) - end, { parent, getOrCreateLoader } :: { unknown }) - - React.useEffect(function() + useEffect(function() local function reloadIfStorybook(instance: Instance) if isStorybookModule(instance) then - loadStorybooks() + loadAllStorybooks() end end - local connections = { - parent.DescendantAdded:Connect(reloadIfStorybook), - } - - for _, loader in loaders.current do - table.insert(connections, loader.loadedModuleChanged:Connect(reloadIfStorybook)) - end + table.insert(connections.current, parent.DescendantAdded:Connect(reloadIfStorybook)) - loadStorybooks() + loadAllStorybooks() return function() - for _, conn in connections do - conn:Disconnect() + for _, connection in connections.current do + connection:Disconnect() end end - end, { loadStorybooks, parent } :: { unknown }) + end, { parent } :: { unknown }) return { available = storybooks, diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index 43ef7b3..d3251cf 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -38,10 +38,9 @@ test("loads storybook modules", function() local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) end) @@ -59,12 +58,11 @@ test("storybooks with syntax errors are marked unavailable", function() local storybooks = get() - expect(storybooks.unavailable).toEqual({ + expect(storybooks.unavailable).toMatchObject({ { problem = expect.any("string"), storybook = { name = storybookModule.Name, - storyRoots = {}, }, }, }) @@ -87,10 +85,9 @@ test("adding a new storybook to the DM triggers a re-render", function() local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) @@ -100,14 +97,12 @@ test("adding a new storybook to the DM triggers a re-render", function() storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, { name = "Foo", - storyRoots = expect.anything(), }, }) end) @@ -127,17 +122,16 @@ test.skip("removing a storybook from the DM triggers a re-render", function() expect(storybooks.available).toEqual({}) end) -test("re-renders on module changes", function() +test.only("re-renders on module changes", function() local get = renderHook(function() return useStorybooks(container) end) local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) @@ -154,10 +148,9 @@ test("re-renders on module changes", function() storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "New Name", - storyRoots = expect.anything(), }, }) end) diff --git a/src/init.luau b/src/init.luau index acc1851..d3acb01 100644 --- a/src/init.luau +++ b/src/init.luau @@ -22,10 +22,10 @@ export type Story = types.Story export type LoadedStory = types.LoadedStory export type Storybook = types.Storybook export type LoadedStorybook = types.LoadedStorybook +export type UnavailableStorybook = types.UnavailableStorybook export type StoryControls = types.StoryControls export type StoryProps = types.StoryProps export type StoryRenderer = types.StoryRenderer -export type UnavailableStorybook = useStorybooks.UnavailableStorybook return { -- Validation diff --git a/src/roblox-internal/isReact.spec.luau b/src/roblox-internal/isReact.spec.luau index 5954a07..5f48e67 100644 --- a/src/roblox-internal/isReact.spec.luau +++ b/src/roblox-internal/isReact.spec.luau @@ -13,14 +13,14 @@ test("React is React", function() expect(isReact(React)).toBeTruthy() end) -test("RoactCompat is not React", function() +test("RoactCompat is React", function() expect(isReact(RoactCompat)).toBeTruthy() end) test("ReactRoblox is not React", function() - expect(isReact(ReactRoblox)).toBeTruthy() + expect(isReact(ReactRoblox)).toBeFalsy() end) test("legacy Roact is not React", function() - expect(isReact(Roact)).toBeTruthy() + expect(isReact(Roact)).toBeFalsy() end) diff --git a/src/types.luau b/src/types.luau index 93b6831..f0d473e 100644 --- a/src/types.luau +++ b/src/types.luau @@ -34,25 +34,14 @@ export type StoryPackages = { [string]: any, } -type MapStory = (props: StoryProps) -> any - -type RobloxInternalStorybook = { - storyRoots: { Instance }, - - name: string?, - mapStory: ((story: any) -> (props: StoryProps) -> any)?, - - roact: any, - react: any, - reactRoblox: any, -} +type MapStoryFn = (story: any) -> (props: StoryProps) -> any export type Storybook = { storyRoots: { Instance }, name: string?, packages: StoryPackages?, - mapStory: ((story: any) -> (props: StoryProps) -> any)?, + mapStory: MapStoryFn?, } types.IStorybook = t.interface({ storyRoots = t.array(t.Instance), @@ -66,9 +55,15 @@ export type LoadedStorybook = { loader: ModuleLoader, source: ModuleScript, + mapStory: MapStoryFn?, packages: StoryPackages?, } +export type UnavailableStorybook = { + problem: string, + storybook: LoadedStorybook, +} + export type Story = { story: T | (props: StoryProps) -> T, From 5026aea829a8dd809e09abfeda23c5e5fd8d7366 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 13:26:57 -0800 Subject: [PATCH 22/38] Support `props` field on Stories --- src/render.luau | 4 ++-- src/types.luau | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/render.luau b/src/render.luau index 1fc56b0..1a169e7 100644 --- a/src/render.luau +++ b/src/render.luau @@ -80,11 +80,11 @@ local function render(container: Instance, story: LoadedStory): RenderLife local prevProps: StoryProps? local function renderOnce(controls: StoryControls?) - local props: StoryProps = { + local props: StoryProps = Sift.Dictionary.join(story.props or {}, { container = container, story = story, theme = "Dark", -- TODO: Support theme changing - } + }) props.controls = Sift.Dictionary.join( if story.controls then collapseControls(story.controls) else nil, diff --git a/src/types.luau b/src/types.luau index f0d473e..ec4e741 100644 --- a/src/types.luau +++ b/src/types.luau @@ -71,6 +71,7 @@ export type Story = { name: string?, packages: StoryPackages?, summary: string?, + props: { [string]: any }?, } types.IStory = t.interface({ story = t.any, @@ -89,6 +90,7 @@ export type LoadedStory = { summary: string?, controls: StoryControls?, + props: { [string]: any }?, } return types From aab4408844f16a42426595be604a326b52335964 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 14:33:07 -0800 Subject: [PATCH 23/38] Support mapDefinition --- src/renderers/createReactRenderer.luau | 8 +++++++- src/renderers/createRoactRenderer.luau | 7 ++++++- src/types.luau | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/renderers/createReactRenderer.luau b/src/renderers/createReactRenderer.luau index d7f38e0..de19128 100644 --- a/src/renderers/createReactRenderer.luau +++ b/src/renderers/createReactRenderer.luau @@ -22,6 +22,13 @@ local function createReactRenderer(packages: Packages): StoryRenderer local currentStory local function reactRender(story: types.LoadedStory, props) + local mapStory = if story.storybook then story.storybook.mapStory else nil + local mapDefinition = if story.storybook then story.storybook.mapDefinition else nil + + if mapDefinition then + story = mapDefinition(story) + end + local element if isReactElement(story.story) then element = story.story @@ -29,7 +36,6 @@ local function createReactRenderer(packages: Packages): StoryRenderer element = React.createElement(story.story, props) end - local mapStory = if story.storybook then story.storybook.mapStory else nil if mapStory then element = React.createElement(mapStory(story.story), props, element) end diff --git a/src/renderers/createRoactRenderer.luau b/src/renderers/createRoactRenderer.luau index 08e0f55..8c3dfac 100644 --- a/src/renderers/createRoactRenderer.luau +++ b/src/renderers/createRoactRenderer.luau @@ -22,8 +22,14 @@ local function createRoactRenderer(packages: Packages): StoryRenderer local currentStory local function mount(container: Instance, story: LoadedStory, props: StoryProps) + local mapStory = if story.storybook then story.storybook.mapStory else nil + local mapDefinition = if story.storybook then story.storybook.mapDefinition else nil local element + if mapDefinition then + story = mapDefinition(story) + end + if isRoactElement(story.story) then currentComponent = (story.story :: any).component element = story.story @@ -32,7 +38,6 @@ local function createRoactRenderer(packages: Packages): StoryRenderer element = Roact.createElement(currentComponent, props) end - local mapStory = if story.storybook then story.storybook.mapStory else nil if mapStory then element = Roact.createElement(mapStory(story.story), props, element) end diff --git a/src/types.luau b/src/types.luau index ec4e741..a285069 100644 --- a/src/types.luau +++ b/src/types.luau @@ -35,6 +35,7 @@ export type StoryPackages = { } type MapStoryFn = (story: any) -> (props: StoryProps) -> any +type MapDefinitionFn = (story: any) -> any export type Storybook = { storyRoots: { Instance }, @@ -42,6 +43,7 @@ export type Storybook = { name: string?, packages: StoryPackages?, mapStory: MapStoryFn?, + mapDefinition: MapDefinitionFn?, } types.IStorybook = t.interface({ storyRoots = t.array(t.Instance), @@ -56,6 +58,7 @@ export type LoadedStorybook = { source: ModuleScript, mapStory: MapStoryFn?, + mapDefinition: MapDefinitionFn?, packages: StoryPackages?, } From 7de020955dd369b878faa2aebb7a6c88d47fa30b Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 14:33:30 -0800 Subject: [PATCH 24/38] Revise useStorybooks to better encapsulate loading --- src/hooks/useStorybooks.luau | 156 ++++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 68 deletions(-) diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 69484d1..3ed90f7 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -74,94 +74,114 @@ local function useStorybooks(parent: Instance): { available: { LoadedStorybook }, unavailable: { UnavailableStorybook }, } - local connections = useRef({} :: { RBXScriptConnection }) + local storybookConnections = useRef({} :: { [ModuleScript]: { RBXScriptConnection } }) local storybooks, setStorybooks = useState({} :: { LoadedStorybook }) local unavailableStorybooks, setUnavailableStorybooks = useState({} :: { UnavailableStorybook }) - local loadStorybook = useCallback(function(storybookModule: ModuleScript, loader: ModuleLoader) - local storybook: LoadedStorybook? - local success, result = pcall(function() - storybook = loadStorybookModule(storybookModule, loader) - end) - - -- TODO: The logic to add/update storybooks at their index is nasty. - -- Revise this before merging - if success and storybook then - setStorybooks(function(prev) - local new = table.clone(prev) - local index = Sift.List.findWhere(new, function(other) - return other.source == storybookModule - end) + local getOrCreateConnectionObject = useCallback(function(storybookModule: ModuleScript): { RBXScriptConnection } + local existing = storybookConnections.current[storybookModule] + if existing then + return existing + else + local new = {} + storybookConnections.current[storybookModule] = new + return new + end + end, {}) - if index then - new[index] = storybook - else - table.insert(new, storybook) - end + local loadStorybook = useCallback(function(storybookModule: ModuleScript) + local connections = getOrCreateConnectionObject(storybookModule) + local loader = ModuleLoader.new() + + local reloadStorybook + + local function load() + for _, connection in connections do + connection:Disconnect() + end - return new + local storybook: LoadedStorybook? + local success, result = pcall(function() + storybook = loadStorybookModule(storybookModule, loader) end) - else - local unavailableStorybook: UnavailableStorybook = { - problem = result, - storybook = { - name = storybookModule.Name, - loader = loader, - source = storybookModule, - storyRoots = {}, - }, - } - - setUnavailableStorybooks(function(prev) - local new = table.clone(prev) - local index = Sift.List.findWhere(new, function(other) - return other.storybook.source == storybookModule + + table.insert(connections, loader.loadedModuleChanged:Connect(reloadStorybook)) + table.insert(connections, storybookModule.AncestryChanged:Connect(reloadStorybook)) + + -- TODO: The logic to add/update storybooks at their index is nasty. + -- Revise this before merging + if success and storybook then + setStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other: LoadedStorybook) + return other.source == storybookModule + end) + + if index then + new[index] = storybook + else + table.insert(new, storybook) + end + + return new end) + else + local unavailableStorybook: UnavailableStorybook = { + problem = result, + storybook = { + name = storybookModule.Name, + loader = loader, + source = storybookModule, + storyRoots = {}, + }, + } + + setUnavailableStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other: UnavailableStorybook) + return other.storybook.source == storybookModule + end) + + if index then + new[index] = unavailableStorybook + else + table.insert(new, unavailableStorybook) + end + + return new + end) + end + end - if index then - new[index] = unavailableStorybook - else - table.insert(new, unavailableStorybook) + function reloadStorybook(instance: Instance) + if instance == storybookModule and isStorybookModule(instance) then + for _, connection in connections do + connection:Disconnect() end - return new - end) + loader:clear() + load() + end end + + load() end, {}) - local loadAllStorybooks = useCallback(function() + useEffect(function() setStorybooks({}) setUnavailableStorybooks({}) - for _, storybookModule in findStorybookModules(parent) do - local loader = ModuleLoader.new() - - table.insert( - connections.current, - loader.loadedModuleChanged:Connect(function() - loader:clear() - loadStorybook(storybookModule, loader) - end) - ) - - loadStorybook(storybookModule, loader) - end - end, { parent }) + local storybookModules = findStorybookModules(parent) - useEffect(function() - local function reloadIfStorybook(instance: Instance) - if isStorybookModule(instance) then - loadAllStorybooks() - end + for _, storybookModule in storybookModules do + task.spawn(loadStorybook, storybookModule) end - table.insert(connections.current, parent.DescendantAdded:Connect(reloadIfStorybook)) - - loadAllStorybooks() - return function() - for _, connection in connections.current do - connection:Disconnect() + for _, connections in storybookConnections.current do + for _, connection in connections do + connection:Disconnect() + end end end end, { parent } :: { unknown }) From 4a99804e30a979a2c2a5a3b505ffc3127985bc3a Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 14:36:21 -0800 Subject: [PATCH 25/38] Provide better interop with storyRoots --- .lune/wally-install.luau | 15 +++++++++++++++ src/loadStorybookModule.luau | 14 ++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/.lune/wally-install.luau b/.lune/wally-install.luau index de9144d..77258d2 100644 --- a/.lune/wally-install.luau +++ b/.lune/wally-install.luau @@ -1,5 +1,20 @@ +local process = require("@lune/process") + local run = require("./lib/run") +local function installPackageFromDisk(packageName: string, packagePath: string) + local homePath = process.env.HOME + assert(homePath, "no $HOME env var") + + local absPackagePath = run("realpath", { packagePath }) + + run("rm", { `Packages/{packageName}.lua` }) + run("ln", { "-s", `{absPackagePath}/dist`, `Packages/{packageName}` }) +end + run("wally", { "install" }) + +installPackageFromDisk("ModuleLoader", "../module-loader") + run("rojo", { "sourcemap", "build.project.json", "-o", "sourcemap.json" }) run("wally-package-types", { "--sourcemap", "sourcemap.json", "Packages" }) diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index f7e2478..2cca50c 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -30,6 +30,20 @@ local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleL assert(wasRequired, `failed to load storybook {module:GetFullName()}: {result}`) + -- Interop with some older storybooks. Ideally this shouldn't be merged + if not result.storyRoots then + if result.storyRoot then + -- Some storybooks use `storyRoot: Instance` instead of a + -- `storyRoots` array. + result.storyRoots = { result.storyRoot } + result.storyRoot = nil + else + -- Some storybooks like AppChat don't supply a storyRoots array. + -- Looks like the default behavior should be to use the parent + result.storyRoots = { module.Parent } + end + end + local isStorybook, message = types.IStorybook(result) assert(isStorybook, `failed to load storybook {module:GetFullName()}:\n{message}`) From c0549811158a8513842661ecc8ba128fb40303ed Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 19:19:30 -0800 Subject: [PATCH 26/38] Each Storybook has its own loader The loader is now instantiated and attached to the storybook implicitly in most cases. This improves sandboxing between storybooks 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 Also keep track of the source module --- src/e2e/e2e.spec.luau | 8 +- src/findStoryModulesForStorybook.spec.luau | 3 + src/hooks/useStory.luau | 39 +++--- src/hooks/useStory.spec.luau | 15 ++- src/hooks/useStorybooks.luau | 133 +++++++++++++++------ src/hooks/useStorybooks.spec.luau | 35 ++---- src/loadStoryModule.luau | 9 +- src/loadStoryModule.spec.luau | 108 +++++++++-------- src/loadStorybookModule.luau | 10 +- src/loadStorybookModule.spec.luau | 12 +- src/render.luau | 5 +- src/test-utils/createStory.luau | 5 + src/types.luau | 7 +- 13 files changed, 231 insertions(+), 158 deletions(-) diff --git a/src/e2e/e2e.spec.luau b/src/e2e/e2e.spec.luau index c2d2797..4ad7b11 100644 --- 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 index a3c384d..8de788f 100644 --- 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 index eb20bd7..1c7fdb1 100644 --- 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,32 +52,27 @@ local types = require("@root/types") @tag React @tag Story @within Storyteller - ]=] -local function useStory( - module: ModuleScript, - storybook: types.LoadedStorybook, - loader: ModuleLoader.ModuleLoader -): (types.LoadedStory?, string?) +local function useStory(module: ModuleScript, storybook: types.LoadedStorybook): (types.LoadedStory?, string?) local state, setState = React.useState({} :: { story: types.LoadedStory?, err: string?, }) - local loadStory = React.useCallback(function() - local story - local success, err = pcall(function() - story = loadStoryModule(loader, module, storybook) - end) - - setState({ - story = if success then story else nil, - err = if not success then err else nil, - }) - end, { loader, module, storybook } :: { unknown }) - React.useEffect(function() - local conn = loader.loadedModuleChanged:Connect(function(other) + local function loadStory() + local story + local success, err = pcall(function() + story = loadStoryModule(module, storybook) + end) + + setState({ + story = if success then story else nil, + err = if not success then err else nil, + }) + end + + local conn = storybook.loader.loadedModuleChanged:Connect(function(other) if other == module then loadStory() end @@ -90,7 +83,7 @@ local function useStory( return function() conn:Disconnect() end - end, { module, loadStory, loader } :: { unknown }) + end, { module, storybook } :: { unknown }) return state.story, state.err end diff --git a/src/hooks/useStory.spec.luau b/src/hooks/useStory.spec.luau index f9a4a51..1c83b88 100644 --- 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,7 +87,7 @@ 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() @@ -118,7 +117,7 @@ end) test("reloads on dependency changes", function() local get = renderHook(function() - return useStory(storyModule, storybook, loader) + return useStory(storyModule, storybook) end) local story = get() diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 9acc24f..64e1979 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -1,11 +1,19 @@ local ModuleLoader = require("@pkg/ModuleLoader") local React = require("@pkg/React") +local Sift = require("@pkg/Sift") local findStorybookModules = require("@root/findStorybookModules") local isStorybookModule = require("@root/isStorybookModule") local loadStorybookModule = require("@root/loadStorybookModule") local types = require("@root/types") +local useCallback = React.useCallback +local useEffect = React.useEffect +local useRef = React.useRef +local useState = React.useState + +type ModuleLoader = ModuleLoader.ModuleLoader +type LoadedStorybook = types.LoadedStorybook type UnavailableStorybook = { problem: string, storybook: types.LoadedStorybook, @@ -32,9 +40,8 @@ type UnavailableStorybook = { local function StorybookList(props: { parent: Instance, - loader: ModuleLoader, }) - local storybooks = Storyteller.useStorybooks(props.parent, props.loader) + local storybooks = Storyteller.useStorybooks(props.parent) local children = {} for index, storybook in storybooks do @@ -66,63 +73,121 @@ type UnavailableStorybook = { @tag Storybook @within Storyteller ]=] -local function useStorybooks( - parent: Instance, - loader: ModuleLoader.ModuleLoader -): { - available: { types.LoadedStorybook }, +local function useStorybooks(parent: Instance): { + available: { LoadedStorybook }, unavailable: { UnavailableStorybook }, } - local storybooks, setStorybooks = React.useState({}) - local unavailableStorybooks, setUnavailableStorybooks = React.useState({} :: { UnavailableStorybook }) + local storybookConnections = useRef({} :: { [ModuleScript]: { RBXScriptConnection } }) + local storybooks, setStorybooks = useState({} :: { LoadedStorybook }) + local unavailableStorybooks, setUnavailableStorybooks = useState({} :: { UnavailableStorybook }) + + local getOrCreateConnectionObject = useCallback(function(storybookModule: ModuleScript): { RBXScriptConnection } + local existing = storybookConnections.current[storybookModule] + if existing then + return existing + else + local new = {} + storybookConnections.current[storybookModule] = new + return new + end + end, {}) + + local loadStorybook = useCallback(function(storybookModule: ModuleScript) + local connections = getOrCreateConnectionObject(storybookModule) + local loader = ModuleLoader.new() + + local reloadStorybook - local loadStorybooks = React.useCallback(function() - local newStorybooks = {} - local newUnavailableStorybooks: { UnavailableStorybook } = {} + local function load() + for _, connection in connections do + connection:Disconnect() + end - for _, storybookModule in findStorybookModules(parent) do - local storybook: types.LoadedStorybook? + local storybook: LoadedStorybook? local success, result = pcall(function() - storybook = loadStorybookModule(loader, storybookModule) + storybook = loadStorybookModule(storybookModule, loader) end) - if success then - table.insert(newStorybooks, storybook) + table.insert(connections, loader.loadedModuleChanged:Connect(reloadStorybook)) + table.insert(connections, storybookModule.AncestryChanged:Connect(reloadStorybook)) + + -- TODO: The logic to add/update storybooks at their index is nasty. + -- Revise this before merging + if success and storybook then + setStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other: LoadedStorybook) + return other.source == storybookModule + end) + + if index then + new[index] = storybook + else + table.insert(new, storybook) + end + + return new + end) else - table.insert(newUnavailableStorybooks, { + local unavailableStorybook: UnavailableStorybook = { problem = result, storybook = { name = storybookModule.Name, + loader = loader, + source = storybookModule, storyRoots = {}, }, - }) + } + + setUnavailableStorybooks(function(prev) + local new = table.clone(prev) + local index = Sift.List.findWhere(new, function(other: UnavailableStorybook) + return other.storybook.source == storybookModule + end) + + if index then + new[index] = unavailableStorybook + else + table.insert(new, unavailableStorybook) + end + + return new + end) end end - setStorybooks(newStorybooks) - setUnavailableStorybooks(newUnavailableStorybooks) - end, { parent, loader } :: { unknown }) + function reloadStorybook(instance: Instance) + if instance == storybookModule and isStorybookModule(instance) then + for _, connection in connections do + connection:Disconnect() + end - React.useEffect(function() - local function reloadIfStorybook(instance: Instance) - if isStorybookModule(instance) then - loadStorybooks() + loader:clear() + load() end end - local connections = { - loader.loadedModuleChanged:Connect(reloadIfStorybook), - parent.DescendantAdded:Connect(reloadIfStorybook), - } + load() + end, {}) + + useEffect(function() + setStorybooks({}) + setUnavailableStorybooks({}) - loadStorybooks() + local storybookModules = findStorybookModules(parent) + + for _, storybookModule in storybookModules do + task.spawn(loadStorybook, storybookModule) + end return function() - for _, conn in connections do - conn:Disconnect() + for _, connections in storybookConnections.current do + for _, connection in connections do + connection:Disconnect() + end end end - end, { loadStorybooks, loader, parent } :: { unknown }) + end, { parent } :: { unknown }) return { available = storybooks, diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index bce574b..d3251cf 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -1,5 +1,4 @@ local JestGlobals = require("@pkg/JestGlobals") -local ModuleLoader = require("@pkg/ModuleLoader") local ReactRoblox = require("@pkg/ReactRoblox") local renderHook = require("@root/test-utils/renderHook") @@ -14,13 +13,10 @@ local act = ReactRoblox.act type LoadedStorybook = types.LoadedStorybook -local loader: ModuleLoader.ModuleLoader local container: Folder local storybookModule: ModuleScript beforeEach(function() - loader = ModuleLoader.new() - container = Instance.new("Folder") storybookModule = Instance.new("ModuleScript") @@ -37,15 +33,14 @@ end) test("loads storybook modules", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) end) @@ -58,17 +53,16 @@ test("storybooks with syntax errors are marked unavailable", function() ]] local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() - expect(storybooks.unavailable).toEqual({ + expect(storybooks.unavailable).toMatchObject({ { problem = expect.any("string"), storybook = { name = storybookModule.Name, - storyRoots = {}, }, }, }) @@ -86,15 +80,14 @@ test("adding a new storybook to the DM triggers a re-render", function() ]] local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) @@ -104,14 +97,12 @@ test("adding a new storybook to the DM triggers a re-render", function() storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, { name = "Foo", - storyRoots = expect.anything(), }, }) end) @@ -119,7 +110,7 @@ end) -- TOOD: Update useStorybooks so this test can pass test.skip("removing a storybook from the DM triggers a re-render", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) act(function() @@ -131,17 +122,16 @@ test.skip("removing a storybook from the DM triggers a re-render", function() expect(storybooks.available).toEqual({}) end) -test("re-renders on module changes", function() +test.only("re-renders on module changes", function() local get = renderHook(function() - return useStorybooks(container, loader) + return useStorybooks(container) end) local storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "Sample", - storyRoots = expect.anything(), }, }) @@ -158,10 +148,9 @@ test("re-renders on module changes", function() storybooks = get() - expect(storybooks.available).toEqual({ + expect(storybooks.available).toMatchObject({ { name = "New Name", - storyRoots = expect.anything(), }, }) end) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 3fae3e5..3c459e7 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -1,4 +1,3 @@ -local ModuleLoader = require("@pkg/ModuleLoader") local Sift = require("@pkg/Sift") local constants = require("@root/constants") @@ -28,13 +27,9 @@ type LoadedStory = types.LoadedStory @tag Module Loading @within Storyteller ]=] -local function loadStoryModule( - loader: ModuleLoader.ModuleLoader, - storyModule: ModuleScript, - storybook: LoadedStorybook -): LoadedStory +local function loadStoryModule(storyModule: ModuleScript, storybook: LoadedStorybook): LoadedStory local success, result = pcall(function() - return loader:require(storyModule) + return storybook.loader:require(storyModule) end) if not success then diff --git a/src/loadStoryModule.spec.luau b/src/loadStoryModule.spec.luau index fc02414..0369d01 100644 --- a/src/loadStoryModule.spec.luau +++ b/src/loadStoryModule.spec.luau @@ -8,36 +8,54 @@ local describe = JestGlobals.describe local jest = JestGlobals.jest local test = JestGlobals.test local expect = JestGlobals.expect - -local MOCK_PLAIN_STORYBOOK: types.LoadedStorybook = { - name = "Plain Storybook", - storyRoots = {}, -} - -local MOCK_ROACT_STORYBOOK: types.LoadedStorybook = { - name = "Roact Storybook", - storyRoots = {}, - packages = { - Roact = { - createElement = jest.fn(), - mount = jest.fn(), - unmount = jest.fn(), - }, - }, -} - -local MOCK_REACT_STORYBOOK: types.LoadedStorybook = { - name = "React Storybook", - storyRoots = {}, - packages = { - React = { - createElement = jest.fn(), +local beforeEach = JestGlobals.beforeEach +local afterEach = JestGlobals.afterEach + +local mockPlainStorybook: types.LoadedStorybook +local mockRoactStorybook: types.LoadedStorybook +local mockReactStorybook: types.LoadedStorybook + +beforeEach(function() + mockPlainStorybook = { + name = "Plain Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), + } + + mockRoactStorybook = { + name = "Roact Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), + packages = { + Roact = { + createElement = jest.fn(), + mount = jest.fn(), + unmount = jest.fn(), + }, }, - ReactRoblox = { - createRoot = jest.fn(), + } + + mockReactStorybook = { + name = "React Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), + packages = { + React = { + createElement = jest.fn(), + }, + ReactRoblox = { + createRoot = jest.fn(), + }, }, - }, -} + } +end) + +afterEach(function() + jest.resetAllMocks() +end) local function createMockStoryModule(source: string): ModuleScript local storyModule = Instance.new("ModuleScript") @@ -48,7 +66,6 @@ local function createMockStoryModule(source: string): ModuleScript end test("load a story module as a table", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { name = "Sample", @@ -56,18 +73,17 @@ test("load a story module as a table", function() } ]]) - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) expect(story).toEqual({ name = "Sample", source = expect.anything(), story = expect.any("function"), - storybook = MOCK_PLAIN_STORYBOOK, + storybook = mockPlainStorybook, }) end) test("handle Hoarcekat stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return function(target) local gui = Instance.new("TextLabel") @@ -79,13 +95,12 @@ test("handle Hoarcekat stories", function() end ]]) - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) expect(story).toBeDefined() end) test("use the name of the story module for the story name", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { story = function() end @@ -93,31 +108,29 @@ test("use the name of the story module for the story name", function() ]]) storyModule.Name = "SampleName.story" - local story = loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + local story = loadStoryModule(storyModule, mockPlainStorybook) assert(story ~= nil, "story not defined") expect(story.name).toEqual("SampleName") end) test("pass the storybook's renderer to the story", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { story = function() end } ]]) - local story = loadStoryModule(loader, storyModule, MOCK_REACT_STORYBOOK) + local story = loadStoryModule(storyModule, mockReactStorybook) expect(story).toBeDefined() - story = loadStoryModule(loader, storyModule, MOCK_ROACT_STORYBOOK) + story = loadStoryModule(storyModule, mockRoactStorybook) expect(story).toBeDefined() end) test("generic failures for stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ syntax error return { @@ -126,12 +139,11 @@ test("generic failures for stories", function() ]]) expect(function() - loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + loadStoryModule(storyModule, mockPlainStorybook) end).toThrow("Failed to load story") end) test("malformed stories", function() - local loader = ModuleLoader.new() local storyModule = createMockStoryModule([[ return { name = false, @@ -140,17 +152,17 @@ test("malformed stories", function() ]]) expect(function() - loadStoryModule(loader, storyModule, MOCK_PLAIN_STORYBOOK) + loadStoryModule(storyModule, mockPlainStorybook) end).toThrow("Story is malformed") end) describe("legacy support", function() test("packages attached to the story get grouped in `packages` object", function() - local loader = ModuleLoader.new() - local storybook: types.LoadedStorybook = { name = "Storybook", storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), } local storyModule = createMockStoryModule([[ @@ -161,7 +173,7 @@ describe("legacy support", function() } ]]) - local story = loadStoryModule(loader, storyModule, storybook) + local story = loadStoryModule(storyModule, storybook) expect(story).toMatchObject({ story = expect.anything(), @@ -173,11 +185,11 @@ describe("legacy support", function() end) test("packages attached to the story take precedence over the storybook", function() - local loader = ModuleLoader.new() - local storybook: types.LoadedStorybook = { name = "Storybook", storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), packages = { Roact = {}, }, @@ -191,7 +203,7 @@ describe("legacy support", function() } ]]) - local story = loadStoryModule(loader, storyModule, storybook) + local story = loadStoryModule(storyModule, storybook) expect(story).toMatchObject({ story = expect.anything(), diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index 956bad8..17f5279 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -4,6 +4,9 @@ local constants = require("@root/constants") local migrateLegacyPackages = require("@root/migrateLegacyPackages") local types = require("@root/types") +type LoadedStorybook = types.LoadedStorybook +type ModuleLoader = ModuleLoader.ModuleLoader + --[=[ Loads the source of a Storybook module. @@ -18,7 +21,9 @@ local types = require("@root/types") @tag Module Loading @within Storyteller ]=] -local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: ModuleScript): types.LoadedStorybook +local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleLoader?): LoadedStorybook + local loader = if providedLoader then providedLoader else ModuleLoader.new() + local wasRequired, result = pcall(function() return loader:require(module) end) @@ -42,6 +47,9 @@ local function loadStorybookModule(loader: ModuleLoader.ModuleLoader, module: Mo end end + result.source = module + result.loader = loader + return result end diff --git a/src/loadStorybookModule.spec.luau b/src/loadStorybookModule.spec.luau index 52a8f75..3b196f8 100644 --- a/src/loadStorybookModule.spec.luau +++ b/src/loadStorybookModule.spec.luau @@ -26,7 +26,7 @@ test("load a storybook module as a table", function() } ]]) - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("Sample") expect(#storybook.storyRoots).toBe(1) @@ -41,7 +41,7 @@ test("use the name of the storybook module by default", function() ]]) storybookModule.Name = "SampleStorybook.storybook" - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("SampleStorybook") end) @@ -55,7 +55,7 @@ test("nameless storybook", function() ]]) storybookModule.Name = ".storybook" - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook.name).toBe("Unnamed Storybook") end) @@ -68,7 +68,7 @@ test("generic failures for storybook", function() ]]) local success, result = pcall(function() - return loadStorybookModule(loader, storybookModule) + return loadStorybookModule(storybookModule, loader) end) expect(success).toBeFalsy() @@ -85,7 +85,7 @@ test("malformed storybook", function() ]]) local success, result = pcall(function() - return loadStorybookModule(loader, storybookModule) + return loadStorybookModule(storybookModule, loader) end) expect(success).toBeFalsy() @@ -106,7 +106,7 @@ describe("legacy support", function() } ]]) - local storybook = loadStorybookModule(loader, storybookModule) + local storybook = loadStorybookModule(storybookModule, loader) expect(storybook).toMatchObject({ storyRoots = expect.anything(), diff --git a/src/render.luau b/src/render.luau index 2136641..1fc56b0 100644 --- a/src/render.luau +++ b/src/render.luau @@ -37,7 +37,6 @@ end local ModuleLoader = require("@pkg/ModuleLoader") local Storyteller = require("@pkg/Storyteller") - local loader = ModuleLoader.new() -- At least one `.storybook` module must be present local storybookModules = Storyteller.findStorybookModules(game) @@ -45,7 +44,7 @@ end local storybook pcall(function() - storybook = Storyteller.loadStorybookModule(loader, storybookModules[1]) + storybook = Storyteller.loadStorybookModule(storybookModules[1]) end) if storybook then @@ -56,7 +55,7 @@ end local story pcall(function() - story = Storyteller.loadStoryModule(loader, storyModules[1], storybook) + story = Storyteller.loadStoryModule(storyModules[1], storybook) end) if story then diff --git a/src/test-utils/createStory.luau b/src/test-utils/createStory.luau index fdf6b36..00cc789 100644 --- a/src/test-utils/createStory.luau +++ b/src/test-utils/createStory.luau @@ -1,3 +1,5 @@ +local ModuleLoader = require("@pkg/ModuleLoader") + local types = require("@root/types") local function createStory(element: T, packages: types.StoryPackages?): types.LoadedStory @@ -6,6 +8,9 @@ local function createStory(element: T, packages: types.StoryPackages?): types story = element, source = Instance.new("ModuleScript"), storybook = { + name = "Storybook", + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), storyRoots = {}, }, packages = packages, diff --git a/src/types.luau b/src/types.luau index 580a2a8..f4068c0 100644 --- a/src/types.luau +++ b/src/types.luau @@ -1,5 +1,8 @@ +local ModuleLoader = require("@pkg/ModuleLoader") local t = require("@pkg/t") +type ModuleLoader = ModuleLoader.ModuleLoader + local types = {} export type StudioTheme = "Light" | "Dark" @@ -46,6 +49,8 @@ types.IStorybook = t.interface({ export type LoadedStorybook = { name: string, storyRoots: { Instance }, + loader: ModuleLoader, + source: ModuleScript, packages: StoryPackages?, } @@ -70,7 +75,7 @@ export type LoadedStory = { name: string, story: T | (props: StoryProps) -> T, source: ModuleScript, - storybook: Storybook, + storybook: LoadedStorybook, packages: StoryPackages?, summary: string?, From c7267cc3ab784276ef431cfa52b6f991aaa30a75 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 19:26:32 -0800 Subject: [PATCH 27/38] Typecasts --- src/hooks/useStorybooks.spec.luau | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index d3251cf..454c976 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -42,7 +42,7 @@ test("loads storybook modules", function() { name = "Sample", }, - }) + } :: any) end) test("storybooks with syntax errors are marked unavailable", function() @@ -65,7 +65,7 @@ test("storybooks with syntax errors are marked unavailable", function() name = storybookModule.Name, }, }, - }) + } :: any) end) test("adding a new storybook to the DM triggers a re-render", function() @@ -89,7 +89,7 @@ test("adding a new storybook to the DM triggers a re-render", function() { name = "Sample", }, - }) + } :: any) act(function() newStorybookModule.Parent = container @@ -104,7 +104,7 @@ test("adding a new storybook to the DM triggers a re-render", function() { name = "Foo", }, - }) + } :: any) end) -- TOOD: Update useStorybooks so this test can pass @@ -122,7 +122,7 @@ test.skip("removing a storybook from the DM triggers a re-render", function() expect(storybooks.available).toEqual({}) end) -test.only("re-renders on module changes", function() +test("re-renders on module changes", function() local get = renderHook(function() return useStorybooks(container) end) @@ -133,7 +133,7 @@ test.only("re-renders on module changes", function() { name = "Sample", }, - }) + } :: any) act(function() storybookModule.Source = [[ @@ -152,5 +152,5 @@ test.only("re-renders on module changes", function() { name = "New Name", }, - }) + } :: any) end) From 113dd992ac6af23ed16747fd9b6ca59ed26d077f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 19:29:47 -0800 Subject: [PATCH 28/38] Couple things to revert --- .lune/wally-install.luau | 15 --------------- src/e2e/storybooks/RobloxInternal.storybook.luau | 15 --------------- 2 files changed, 30 deletions(-) delete mode 100644 src/e2e/storybooks/RobloxInternal.storybook.luau diff --git a/.lune/wally-install.luau b/.lune/wally-install.luau index 77258d2..de9144d 100644 --- a/.lune/wally-install.luau +++ b/.lune/wally-install.luau @@ -1,20 +1,5 @@ -local process = require("@lune/process") - local run = require("./lib/run") -local function installPackageFromDisk(packageName: string, packagePath: string) - local homePath = process.env.HOME - assert(homePath, "no $HOME env var") - - local absPackagePath = run("realpath", { packagePath }) - - run("rm", { `Packages/{packageName}.lua` }) - run("ln", { "-s", `{absPackagePath}/dist`, `Packages/{packageName}` }) -end - run("wally", { "install" }) - -installPackageFromDisk("ModuleLoader", "../module-loader") - run("rojo", { "sourcemap", "build.project.json", "-o", "sourcemap.json" }) run("wally-package-types", { "--sourcemap", "sourcemap.json", "Packages" }) diff --git a/src/e2e/storybooks/RobloxInternal.storybook.luau b/src/e2e/storybooks/RobloxInternal.storybook.luau deleted file mode 100644 index 9a9780c..0000000 --- a/src/e2e/storybooks/RobloxInternal.storybook.luau +++ /dev/null @@ -1,15 +0,0 @@ -local RoactCompat = require("@pkg/RoactCompat") - -return { - roact = RoactCompat, - storyRoots = { - script.Parent.stories.react, - }, - mapStory = function(story) - return function(storyProps) - return RoactCompat.createElement("Frame", { - BackgroundColor3 = Color3.fromRGB(255, 0, 0), - }, RoactCompat.createElement(story, storyProps)) - end - end, -} From 1e42f866f9415043fd002af785d79d1dc014e8fe Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Fri, 20 Dec 2024 16:35:38 -0800 Subject: [PATCH 29/38] Add some more tests --- src/hooks/useStorybooks.spec.luau | 38 ++++++++++++++--------------- src/loadStoryModule.spec.luau | 29 ++++++++++++++++++++++ src/loadStorybookModule.spec.luau | 33 +++++++++++++++++++++++++ src/migrateLegacyPackages.spec.luau | 34 ++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 19 deletions(-) create mode 100644 src/migrateLegacyPackages.spec.luau diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index d3251cf..df3cf34 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -74,7 +74,7 @@ test("adding a new storybook to the DM triggers a re-render", function() newStorybookModule.Source = [[ return { storyRoots = { - script.Parent + Instance.new("Folder") } } ]] @@ -85,30 +85,30 @@ test("adding a new storybook to the DM triggers a re-render", function() local storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "Sample", - }, + }), }) act(function() newStorybookModule.Parent = container + task.wait() end) storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "Sample", - }, - { + }), + expect.objectContaining({ name = "Foo", - }, + }), }) end) --- TOOD: Update useStorybooks so this test can pass -test.skip("removing a storybook from the DM triggers a re-render", function() +test("removing a storybook from the DM triggers a re-render", function() local get = renderHook(function() return useStorybooks(container) end) @@ -122,17 +122,17 @@ test.skip("removing a storybook from the DM triggers a re-render", function() expect(storybooks.available).toEqual({}) end) -test.only("re-renders on module changes", function() +test("re-renders on module changes", function() local get = renderHook(function() return useStorybooks(container) end) local storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "Sample", - }, + }), }) act(function() @@ -140,7 +140,7 @@ test.only("re-renders on module changes", function() return { name = "New Name", storyRoots = { - script.Parent + Instance.new("Folder") } } ]] @@ -148,9 +148,9 @@ test.only("re-renders on module changes", function() storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "New Name", - }, + }), }) end) diff --git a/src/loadStoryModule.spec.luau b/src/loadStoryModule.spec.luau index 0369d01..d821a86 100644 --- a/src/loadStoryModule.spec.luau +++ b/src/loadStoryModule.spec.luau @@ -218,3 +218,32 @@ describe("legacy support", function() }) end) end) + +describe("roblox internal", function() + -- Right now we don't support rendering more than one story. For now, we + -- pick a random one to render. + test("pick a single substory to show", function() + local storybook: types.LoadedStorybook = { + name = "Storybook", + storyRoots = {}, + loader = ModuleLoader.new(), + source = Instance.new("ModuleScript"), + packages = {}, + } + + local storyModule = createMockStoryModule([[ + return { + stories = { + One = function() end, + Two = function() end, + Three = function() end, + } + } + ]]) + + local story = loadStoryModule(storyModule, storybook) + + expect(story.story).toBeDefined() + expect((story :: any).substories).toBeUndefined() + end) +end) diff --git a/src/loadStorybookModule.spec.luau b/src/loadStorybookModule.spec.luau index 3b196f8..69c2fff 100644 --- a/src/loadStorybookModule.spec.luau +++ b/src/loadStorybookModule.spec.luau @@ -117,3 +117,36 @@ describe("legacy support", function() }) end) end) + +describe("roblox internal", function() + test("map storyRoot (single instance) to storyRoots (array of instances)", function() + local storybookModule = createMockStorybookModule([[ + return { + storyRoot = Instance.new("Folder"), + } + ]]) + + local storybook = loadStorybookModule(storybookModule) + + expect(#storybook.storyRoots).toBe(1) + end) + + test("use the story module's parent when no story roots are specified", function() + local parent = Instance.new("Folder") + + local storybookModule = createMockStorybookModule([[ + return { + name = "Sample", + } + ]]) + storybookModule.Parent = parent + + local storybook = loadStorybookModule(storybookModule) + + expect(storybook).toMatchObject({ + storyRoots = { + parent, + }, + }) + end) +end) diff --git a/src/migrateLegacyPackages.spec.luau b/src/migrateLegacyPackages.spec.luau new file mode 100644 index 0000000..ef4f69c --- /dev/null +++ b/src/migrateLegacyPackages.spec.luau @@ -0,0 +1,34 @@ +local JestGlobals = require("@pkg/JestGlobals") +local React = require("@pkg/React") +local ReactRoblox = require("@pkg/ReactRoblox") +local Roact = require("@pkg/Roact") + +local migrateLegacyPackages = require("./migrateLegacyPackages") + +local describe = JestGlobals.describe +local test = JestGlobals.test +local expect = JestGlobals.expect + +test("map roact key", function() + local packages = migrateLegacyPackages({ + roact = Roact, + }) + + expect(packages).toEqual({ + Roact = Roact, + }) +end) + +describe("roblox internal", function() + test("react can be passed as the roact key", function() + local packages = migrateLegacyPackages({ + roact = React, + reactRoblox = ReactRoblox, + }) + + expect(packages).toEqual({ + React = React, + ReactRoblox = ReactRoblox, + }) + end) +end) From 05cafb2c6d47eef64a6c5eca34958cc6f65b8982 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 23 Dec 2024 16:47:52 -0800 Subject: [PATCH 30/38] Unskip a test --- src/hooks/useStorybooks.spec.luau | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index 454c976..7faa0b6 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -107,8 +107,7 @@ test("adding a new storybook to the DM triggers a re-render", function() } :: any) end) --- TOOD: Update useStorybooks so this test can pass -test.skip("removing a storybook from the DM triggers a re-render", function() +test("removing a storybook from the DM triggers a re-render", function() local get = renderHook(function() return useStorybooks(container) end) From ee649a4b5799d0d8650bd0cb5bad2a17e601665a Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sat, 28 Dec 2024 19:11:04 -0800 Subject: [PATCH 31/38] Get useStorybooks tests passing --- src/hooks/useStorybooks.luau | 120 +++++++++++++++--------------- src/hooks/useStorybooks.spec.luau | 51 +++++++------ 2 files changed, 87 insertions(+), 84 deletions(-) diff --git a/src/hooks/useStorybooks.luau b/src/hooks/useStorybooks.luau index 64e1979..9fd77c3 100644 --- a/src/hooks/useStorybooks.luau +++ b/src/hooks/useStorybooks.luau @@ -1,6 +1,5 @@ local ModuleLoader = require("@pkg/ModuleLoader") local React = require("@pkg/React") -local Sift = require("@pkg/Sift") local findStorybookModules = require("@root/findStorybookModules") local isStorybookModule = require("@root/isStorybookModule") @@ -11,6 +10,7 @@ local useCallback = React.useCallback local useEffect = React.useEffect local useRef = React.useRef local useState = React.useState +local useMemo = React.useMemo type ModuleLoader = ModuleLoader.ModuleLoader type LoadedStorybook = types.LoadedStorybook @@ -78,8 +78,7 @@ local function useStorybooks(parent: Instance): { unavailable: { UnavailableStorybook }, } local storybookConnections = useRef({} :: { [ModuleScript]: { RBXScriptConnection } }) - local storybooks, setStorybooks = useState({} :: { LoadedStorybook }) - local unavailableStorybooks, setUnavailableStorybooks = useState({} :: { UnavailableStorybook }) + local storybooks, setStorybooks = useState({} :: { [ModuleScript]: LoadedStorybook | UnavailableStorybook }) local getOrCreateConnectionObject = useCallback(function(storybookModule: ModuleScript): { RBXScriptConnection } local existing = storybookConnections.current[storybookModule] @@ -99,10 +98,6 @@ local function useStorybooks(parent: Instance): { local reloadStorybook local function load() - for _, connection in connections do - connection:Disconnect() - end - local storybook: LoadedStorybook? local success, result = pcall(function() storybook = loadStorybookModule(storybookModule, loader) @@ -111,59 +106,44 @@ local function useStorybooks(parent: Instance): { table.insert(connections, loader.loadedModuleChanged:Connect(reloadStorybook)) table.insert(connections, storybookModule.AncestryChanged:Connect(reloadStorybook)) - -- TODO: The logic to add/update storybooks at their index is nasty. - -- Revise this before merging - if success and storybook then - setStorybooks(function(prev) - local new = table.clone(prev) - local index = Sift.List.findWhere(new, function(other: LoadedStorybook) - return other.source == storybookModule - end) - - if index then - new[index] = storybook - else - table.insert(new, storybook) - end - - return new - end) - else - local unavailableStorybook: UnavailableStorybook = { - problem = result, - storybook = { - name = storybookModule.Name, - loader = loader, - source = storybookModule, - storyRoots = {}, - }, - } - - setUnavailableStorybooks(function(prev) - local new = table.clone(prev) - local index = Sift.List.findWhere(new, function(other: UnavailableStorybook) - return other.storybook.source == storybookModule - end) - - if index then - new[index] = unavailableStorybook - else - table.insert(new, unavailableStorybook) - end + setStorybooks(function(prev) + local new = table.clone(prev) + + if success and storybook then + new[storybookModule] = storybook + else + local unavailableStorybook: UnavailableStorybook = { + problem = result, + storybook = { + name = storybookModule.Name, + loader = loader, + source = storybookModule, + storyRoots = {}, + }, + } + new[storybookModule] = unavailableStorybook + end - return new - end) - end + return new + end) end function reloadStorybook(instance: Instance) - if instance == storybookModule and isStorybookModule(instance) then - for _, connection in connections do - connection:Disconnect() - end + if instance == storybookModule then + if instance.Parent ~= nil and isStorybookModule(instance) then + for _, connection in connections do + connection:Disconnect() + end - loader:clear() - load() + loader:clear() + load() + else + setStorybooks(function(prev) + local new = table.clone(prev) + new[storybookModule] = nil + return new + end) + end end end @@ -172,7 +152,6 @@ local function useStorybooks(parent: Instance): { useEffect(function() setStorybooks({}) - setUnavailableStorybooks({}) local storybookModules = findStorybookModules(parent) @@ -180,7 +159,15 @@ local function useStorybooks(parent: Instance): { task.spawn(loadStorybook, storybookModule) end + local conn = parent.DescendantAdded:Connect(function(descendant) + if isStorybookModule(descendant) then + loadStorybook(descendant :: ModuleScript) + end + end) + return function() + conn:Disconnect() + for _, connections in storybookConnections.current do for _, connection in connections do connection:Disconnect() @@ -189,10 +176,23 @@ local function useStorybooks(parent: Instance): { end end, { parent } :: { unknown }) - return { - available = storybooks, - unavailable = unavailableStorybooks, - } + return useMemo(function() + local available: { LoadedStorybook } = {} + local unavailable: { UnavailableStorybook } = {} + + for _, storybook in storybooks do + if storybook["problem"] == nil then + table.insert(available, storybook :: LoadedStorybook) + else + table.insert(unavailable, storybook :: UnavailableStorybook) + end + end + + return { + available = available, + unavailable = unavailable, + } + end, { storybooks }) end return useStorybooks diff --git a/src/hooks/useStorybooks.spec.luau b/src/hooks/useStorybooks.spec.luau index 7faa0b6..766a3d4 100644 --- a/src/hooks/useStorybooks.spec.luau +++ b/src/hooks/useStorybooks.spec.luau @@ -42,7 +42,7 @@ test("loads storybook modules", function() { name = "Sample", }, - } :: any) + }) end) test("storybooks with syntax errors are marked unavailable", function() @@ -65,16 +65,16 @@ test("storybooks with syntax errors are marked unavailable", function() name = storybookModule.Name, }, }, - } :: any) + }) end) test("adding a new storybook to the DM triggers a re-render", function() local newStorybookModule = Instance.new("ModuleScript") - newStorybookModule.Name = "Foo.storybook" + newStorybookModule.Name = "BOO.storybook" newStorybookModule.Source = [[ return { storyRoots = { - script.Parent + Instance.new("Folder") } } ]] @@ -85,26 +85,27 @@ test("adding a new storybook to the DM triggers a re-render", function() local storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "Sample", - }, - } :: any) + }), + }) act(function() newStorybookModule.Parent = container + task.wait() end) storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual(expect.arrayContaining({ + expect.objectContaining({ name = "Sample", - }, - { - name = "Foo", - }, - } :: any) + }), + expect.objectContaining({ + name = "BOO", + }), + })) end) test("removing a storybook from the DM triggers a re-render", function() @@ -114,6 +115,7 @@ test("removing a storybook from the DM triggers a re-render", function() act(function() storybookModule:Destroy() + task.wait() end) local storybooks = get() @@ -128,28 +130,29 @@ test("re-renders on module changes", function() local storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "Sample", - }, - } :: any) + }), + }) act(function() storybookModule.Source = [[ return { name = "New Name", storyRoots = { - script.Parent + Instance.new("Folder") } } ]] + task.wait() end) storybooks = get() - expect(storybooks.available).toMatchObject({ - { + expect(storybooks.available).toEqual({ + expect.objectContaining({ name = "New Name", - }, - } :: any) + }), + }) end) From 5c757f6b412289cab44e3e4e3e0dadbed0fe929f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 10:55:19 -0800 Subject: [PATCH 32/38] Get useStory tests passing --- src/hooks/useStory.luau | 41 ++++++++++++++++++++---------------- src/hooks/useStory.spec.luau | 16 ++++++++------ 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/hooks/useStory.luau b/src/hooks/useStory.luau index 1c7fdb1..5c069de 100644 --- a/src/hooks/useStory.luau +++ b/src/hooks/useStory.luau @@ -54,38 +54,43 @@ local types = require("@root/types") @within Storyteller ]=] local function useStory(module: ModuleScript, storybook: types.LoadedStorybook): (types.LoadedStory?, string?) - local state, setState = React.useState({} :: { - story: types.LoadedStory?, - err: string?, - }) + local story, setStory = React.useState(nil :: types.LoadedStory?) + local err, setErr = React.useState(nil :: string?) React.useEffect(function() local function loadStory() - local story - local success, err = pcall(function() - story = loadStoryModule(module, storybook) + local newStory + local success, result = pcall(function() + newStory = loadStoryModule(module, storybook) end) - setState({ - story = if success then story else nil, - err = if not success then err else nil, - }) + setStory(if success then newStory else nil) + setErr(if success then nil else result) end - local conn = storybook.loader.loadedModuleChanged:Connect(function(other) - if other == module then - loadStory() - end - end) + local connections: { RBXScriptConnection } = {} + + 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, 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 index 1c83b88..133661d 100644 --- a/src/hooks/useStory.spec.luau +++ b/src/hooks/useStory.spec.luau @@ -90,10 +90,12 @@ test("re-renders on module changes", function() 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) @@ -109,10 +111,12 @@ 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() From 93e827fdd1a73daa6cbd3a0943314c70d2a56bff Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 11:18:59 -0800 Subject: [PATCH 33/38] Require useStorybooks inline --- src/init.luau | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/init.luau b/src/init.luau index d3acb01..d9f4d92 100644 --- a/src/init.luau +++ b/src/init.luau @@ -15,8 +15,6 @@ local types = require("./types") @class Storyteller ]=] -local useStorybooks = require("./hooks/useStorybooks") - export type RenderLifecycle = types.RenderLifecycle export type Story = types.Story export type LoadedStory = types.LoadedStory @@ -45,5 +43,5 @@ return { -- Hooks useStory = require("./hooks/useStory"), - useStorybooks = useStorybooks, + useStorybooks = require("./hooks/useStorybooks"), } From 722387f6a6dd552e2cb44aa967a9b1542ebce5e1 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 18:41:26 -0800 Subject: [PATCH 34/38] Remove implicit storyRoots --- src/loadStorybookModule.luau | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index 2cca50c..59f0c83 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -31,17 +31,11 @@ local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleL assert(wasRequired, `failed to load storybook {module:GetFullName()}: {result}`) -- Interop with some older storybooks. Ideally this shouldn't be merged - if not result.storyRoots then - if result.storyRoot then - -- Some storybooks use `storyRoot: Instance` instead of a - -- `storyRoots` array. - result.storyRoots = { result.storyRoot } - result.storyRoot = nil - else - -- Some storybooks like AppChat don't supply a storyRoots array. - -- Looks like the default behavior should be to use the parent - result.storyRoots = { module.Parent } - end + if not result.storyRoots and result.storyRoot then + -- Some storybooks use `storyRoot: Instance` instead of a + -- `storyRoots` array. + result.storyRoots = { result.storyRoot } + result.storyRoot = nil end local isStorybook, message = types.IStorybook(result) From e85ae1761c22b5c449fb477a7f87853d1649cedd Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 19:13:18 -0800 Subject: [PATCH 35/38] Mark Roblox internal behavior --- src/loadStorybookModule.luau | 19 +++++++++++-------- src/migrateLegacyPackages.luau | 13 +++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/loadStorybookModule.luau b/src/loadStorybookModule.luau index 59f0c83..9a93df8 100644 --- a/src/loadStorybookModule.luau +++ b/src/loadStorybookModule.luau @@ -30,12 +30,13 @@ local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleL assert(wasRequired, `failed to load storybook {module:GetFullName()}: {result}`) - -- Interop with some older storybooks. Ideally this shouldn't be merged - if not result.storyRoots and result.storyRoot then - -- Some storybooks use `storyRoot: Instance` instead of a - -- `storyRoots` array. - result.storyRoots = { result.storyRoot } - result.storyRoot = nil + do -- Roblox internal. This behavior may be removed without notice. + if not result.storyRoots and result.storyRoot then + -- Some storybooks use `storyRoot: Instance` instead of a + -- `storyRoots` array. + result.storyRoots = { result.storyRoot } + result.storyRoot = nil + end end local isStorybook, message = types.IStorybook(result) @@ -55,8 +56,10 @@ local function loadStorybookModule(module: ModuleScript, providedLoader: ModuleL end end - if result.group then - result.name = `{result.group} / {result.name}` + do -- Roblox internal. This behavior may be removed without notice. + if result.group then + result.name = `{result.group} / {result.name}` + end end result.source = module diff --git a/src/migrateLegacyPackages.luau b/src/migrateLegacyPackages.luau index 9e9a5c5..cebc243 100644 --- a/src/migrateLegacyPackages.luau +++ b/src/migrateLegacyPackages.luau @@ -5,12 +5,13 @@ type StoryPackages = types.StoryPackages local function migrateLegacyPackages(storyOrStorybook: { [string]: any }): StoryPackages? if storyOrStorybook.roact or storyOrStorybook.react or storyOrStorybook.reactRoblox then - -- Developer Storybook - if storyOrStorybook.roact and storyOrStorybook.reactRoblox and isReact(storyOrStorybook.roact) then - return { - React = storyOrStorybook.roact, - ReactRoblox = storyOrStorybook.reactRoblox, - } + do -- Roblox internal. This behavior may be removed without notice. + if storyOrStorybook.roact and storyOrStorybook.reactRoblox and isReact(storyOrStorybook.roact) then + return { + React = storyOrStorybook.roact, + ReactRoblox = storyOrStorybook.reactRoblox, + } + end end return { From c6686408edf49cf0210a2df52b56b9e5a8e41e03 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 19:13:30 -0800 Subject: [PATCH 36/38] Remove test for removed behavior --- src/loadStorybookModule.spec.luau | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/loadStorybookModule.spec.luau b/src/loadStorybookModule.spec.luau index 69c2fff..dffaf38 100644 --- a/src/loadStorybookModule.spec.luau +++ b/src/loadStorybookModule.spec.luau @@ -130,23 +130,4 @@ describe("roblox internal", function() expect(#storybook.storyRoots).toBe(1) end) - - test("use the story module's parent when no story roots are specified", function() - local parent = Instance.new("Folder") - - local storybookModule = createMockStorybookModule([[ - return { - name = "Sample", - } - ]]) - storybookModule.Parent = parent - - local storybook = loadStorybookModule(storybookModule) - - expect(storybook).toMatchObject({ - storyRoots = { - parent, - }, - }) - end) end) From cef8dc1ca49903746e743824d8c5149077c50522 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 19:14:32 -0800 Subject: [PATCH 37/38] Update story format docs --- docs/docs/story-format.md | 117 +++++++++++++------------------------- 1 file changed, 38 insertions(+), 79 deletions(-) diff --git a/docs/docs/story-format.md b/docs/docs/story-format.md index 8f57bb8..eeaf9bf 100644 --- a/docs/docs/story-format.md +++ b/docs/docs/story-format.md @@ -4,28 +4,25 @@ Any ModuleScript with a `.storybook` extension will be picked up as a Storybook. -:::warning -Storybooks that do not have a `storyRoots` array will not be shown in the Flipbook UI. -::: - The properties that can be used in the module are as follows: -| **Property** | **Type** | **Description** | -| ------------ | -------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | -| `storyRoots` | `{ Instance }` | Locations that the Storybook manages. Each instance will have its descendants searched for Story modules. | -| `name` | `string?` | An optional name for the Storybook. Defaults to the module name with the extension removed. i.e. `Sample.storybook` becomes `Sample`. | -| `packages` | `{ [string]: any }?` | An optional dictionary used for supplying the Storybook with the packages to use when rendering its Stories. | +| **Property** | **Type** | **Description** | +| --------------- | ----------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | +| `storyRoots` | `{ Instance }` | Locations that the Storybook manages. Each instance will have its descendants searched for Story modules. | +| `name` | `string?` | An optional name for the Storybook. Defaults to the module name with the extension removed. i.e. `Sample.storybook` becomes `Sample`. | +| `packages` | `{ [string]: any }?` | An optional dictionary used for supplying the Storybook with the packages to use when rendering its Stories. | +| `mapDefinition` | `((story: any) -> any)?` | | +| `mapStory` | `((story: any) -> (props: StoryProps) -> any)?` | | -This dictionary can also be supplied per-Story to change the renderer used, but it can be convenient to define your packages globally to avoid repetition. | +This dictionary can also be supplied per-Story to change the renderer used, but it can be convenient to define your packages globally to avoid repetition. -Example Storybook module: +The most basic Storybook module can be represented as: -```lua --- Sample.storybook +```lua title="Plain.storybook.luau" return { - storyRoots = { - script.Parent.Components - }, + storyRoots = { + script.Parent, + }, } ``` @@ -35,37 +32,28 @@ Any ModuleScript with a `.story` extension will be picked up as a Story when it The only required member of a Story definition is the `story` property. -| **Property** | **Type** | **Description** | -| ------------ | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `story` | `((props: StoryProps) -> T)` | | -| `name` | `string?` | The name of the Story as it appears in Flipbook. Defaults to the name of the Story module. i.e. `Sample.story` becomes `Sample` | -| `summary` | `string?` | A description of the Story that will appear above the rendered preview in Flipbook. | -| `controls` | `{ [string]: any }?` | Controls allow for on-the-fly configuration of your rendered UI. Read more about how to define and use Controls here: [Controls](https://www.notion.so/Controls-12f95b7912f8804388e1d746a6617716?pvs=21) | - -The type of the `story` property is dependent on what kind of Story is being rendered. Flipbook does not prescribe one particular way of writing Stories, or even a particular UI library that must be used. -Stories can be written for React, Fusion, legacy Roact, plain Roblox Instances, and anything you can think of with [Manual Stories](https://www.notion.so/Story-format-12f95b7912f880068da6d74c472bf186?pvs=21). - -Example Story module: - -```lua -return { - story = function(props) - - end -} -``` - -Under the hood these simply map to `packages.Roact`, `packages.React`, and `packages.ReactRoblox` respectively. +| **Property** | **Type** | **Description** | +| ------------ | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `story` | `((props: StoryProps) -> T)` | | +| `name` | `string?` | The name of the Story as it appears in Storyteller. Defaults to the name of the Story module. i.e. `Sample.story` becomes `Sample` | +| `summary` | `string?` | A description of the Story that will appear above the rendered preview in Storyteller. | +| `controls` | `{ [string]: any }?` | Controls allow for on-the-fly configuration of your rendered UI. Read more about how to define and use controls [here](/docs/creating-stories/controls). | +| `packages` | `{ [string]: any }?` | An optional dictionary used for supplying the Story with the packages to use when rendering. The Story inherits the packages defined by the Storybook, so this is mostly used in cases where Story needs to deviate from the usual renderer | +| `props` | `{ [string]: any }?` | | -## Stories for different renderers +The type of the `story` property is dependent on what kind of Story is being rendered. Storyteller does not prescribe one particular way of writing Stories, or even a particular UI library that must be used. -### Manual +## StoryProps -### React +A Story's `story` function is passed in a `StoryProps` object that contains the following. -### Roact +| **Property** | **Type** | **Description** | +| ------------ | --------------- | ------------------------------------------------------ | +| `container` | `Instance` | | +| `theme` | `string` | A string representing the current Roblox Studio theme. | +| `controls` | `StoryControls` | Defaults to an empty table. | -### Fusion +If `props` is supplied on the Story, then the key/value pairs will be merged with `StoryProps`. ## Legacy package support @@ -73,42 +61,13 @@ Under the hood these simply map to `packages.Roact`, `packages.React`, and `pack A future version of Storyteller may remove this compatibility layer. It is recommended to migrate to `packages` in the meantime. ::: -For UI libraries to be used by Flipbook it was required to pass them as properties to the Story or Storybook. For example, to use React with Flipbook you would define a Storybook like the following: - -```lua -local React = require("@pkg/React") -local ReactRoblox = require("@pkg/ReactRoblox") - -return { - storyRoots = { - script.Parent.Components, - } - react = React, - reactRoblox = ReactRoblox, -} -``` - -Storyteller uses a different approach by grouping any third-party libraries used for rendering stories in a `packages` object. - -```lua -local React = require("@pkg/React") -local ReactRoblox = require("@pkg/ReactRoblox") - -return { - storyRoots = { - script.Parent.Components, - } - packages = { - React = React, - ReactRoblox = ReactRoblox, - } -} -``` +UI libraries used to be supplied by attaching them as properties to a Story or Storybook. This has been superseded by the `packages` object, which acts as a dedicated location to supply the packages used for rendering Stories. -For convenience, Storyteller provides backwards compatibility for the following Storybook properties: +For backwards compatibility, the following properties are migrated to their `packages` equivalent: -| **Property** | **Type** | **Description** | -| ------------- | -------- | ----------------------------------------------------------- | -| `roact` | `any` | Result of `require(ReplicatedStorage.Packages.Roact)` | -| `react` | `any` | Result of `require(ReplicatedStorage.Packages.React)` | -| `reactRoblox` | `any` | Result of `require(ReplicatedStorage.Packages.ReactRoblox)` | +| **Property** | **Mapping** | +| ------------- | ---------------------- | +| `fusion` | `packages.Fusion` | +| `react` | `packages.React` | +| `reactRoblox` | `packages.ReactRoblox` | +| `roact` | `packages.Roact` | From 182fe0b0b7f6ed29d17ee0ecb772528316613919 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 19:16:29 -0800 Subject: [PATCH 38/38] Mark another Roblox internal feature --- src/loadStoryModule.luau | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/loadStoryModule.luau b/src/loadStoryModule.luau index 28a1db5..37c87cf 100644 --- a/src/loadStoryModule.luau +++ b/src/loadStoryModule.luau @@ -48,19 +48,21 @@ local function loadStoryModule(storyModule: ModuleScript, storybook: LoadedSt } end - -- TODO: We need support for substories to be compatible with Developer - -- Storybook. In the meantime we're just selecting a randomly accessed - -- one - if result.stories and not result.story then - local randomSubstory - for _, substory in result.stories do - randomSubstory = substory - break - end - if typeof(randomSubstory) == "table" then - result.story = randomSubstory.story - else - result.story = randomSubstory + do -- Roblox internal. This behavior may be substantially changed or removed + -- We need support for substories to be compatible with Developer + -- Storybook. In the meantime we're just selecting a randomly accessed + -- one, but ideally we should be able to render them all out + if result.stories and not result.story then + local randomSubstory + for _, substory in result.stories do + randomSubstory = substory + break + end + if typeof(randomSubstory) == "table" then + result.story = randomSubstory.story + else + result.story = randomSubstory + end end end