From 23b8962bc3bc320a136a466f1048e8ae18bf9348 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 6 Nov 2024 14:49:39 -0800 Subject: [PATCH 01/28] Add loadmodule support --- src/ModuleLoader.luau | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 719e7d3..d3b80df 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -6,6 +6,9 @@ local getCallerPath = require("./getCallerPath") local getEnv = require("./getEnv") local getRobloxTsRuntime = require("./getRobloxTsRuntime") +local loadmodule = (debug :: any).loadmodule +local loadmoduleEnabled = pcall(loadmodule, Instance.new("ModuleScript")) + type CachedModuleResult = any type ModuleConsumers = { @@ -222,7 +225,13 @@ function ModuleLoader:require(module: ModuleScript) end local source = self:_getSource(module) - local moduleFn, parseError = self._loadstring(source, module:GetFullName()) + local moduleFn, parseError + + if loadmoduleEnabled then + moduleFn, parseError = loadmodule(module) + else + moduleFn, parseError = self._loadstring(source, module:GetFullName()) + end if not moduleFn then local message = if parseError then parseError else "" From edd13455ed1eae4079398bb3730521fa73f485cc Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 12:57:43 -0800 Subject: [PATCH 02/28] Fix building --- .lune/build.luau | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.lune/build.luau b/.lune/build.luau index 6251a99..d8082f2 100644 --- a/.lune/build.luau +++ b/.lune/build.luau @@ -15,7 +15,7 @@ local function build() compile(target) if target == "prod" then - run("rm", { "-rf", `{project.BUILD_PATH}/**/*.spec.luau` }) + run("find", { project.BUILD_PATH, "-name", "*.spec.luau", "-type", "f", "-delete" }) end end @@ -25,7 +25,6 @@ if args.watch then watch({ filePatterns = { "src/.*%.luau", - "example/.*%.luau", }, onChanged = build, }) From 55c2ed9b512752bc5203a7353370b5a2f4e63b3f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 15:26:14 -0800 Subject: [PATCH 03/28] Use `test` for jest tests --- src/ModuleLoader.spec.luau | 64 +++++++++++++++------------- src/bind.spec.luau | 6 +-- src/createTablePassthrough.spec.luau | 4 +- src/getEnv.spec.luau | 8 ++-- src/getRobloxTsRuntime.spec.luau | 6 +-- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index 04382e9..c53df5e 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -1,7 +1,7 @@ local ReplicatedStorage = game:GetService("ReplicatedStorage") local JestGlobals = require("@pkg/JestGlobals") -local it = JestGlobals.it +local test = JestGlobals.test local expect = JestGlobals.expect local describe = JestGlobals.describe local beforeEach = JestGlobals.beforeEach @@ -84,7 +84,7 @@ describe("_getSource", function() end) describe("_trackChanges", function() - it("should create a Janitor instance if it doesn't exist", function() + test("creates a Janitor instance if it doesn't exist", function() local mockModuleInstance = Instance.new("ModuleScript") expect(loader._janitors[mockModuleInstance.Name]).toBeUndefined() @@ -94,7 +94,7 @@ describe("_trackChanges", function() expect(loader._janitors[mockModuleInstance.Name]).toBeDefined() end) - it("should reuse the same Janitor instance for future calls", function() + test("reuses the same Janitor instance for future calls", function() local mockModuleInstance = Instance.new("ModuleScript") loader:_trackChanges(mockModuleInstance) @@ -108,7 +108,7 @@ describe("_trackChanges", function() end) describe("loadedModuleChanged", function() - it("should fire when a required module has its ancestry changed", function() + test("fires when a required module has its ancestry changed", function() local mockModuleInstance = Instance.new("ModuleScript") local wasFired = false @@ -126,13 +126,15 @@ describe("loadedModuleChanged", function() -- Require the module so that events get setup loader:require(mockModuleInstance) + expect(wasFired).toBe(false) + -- Trigger AncestryChanged to fire mockModuleInstance.Parent = nil expect(wasFired).toBe(true) end) - it("should fire when a required module has its Source property change", function() + test("fires when a required module has its Source property change", function() local mockModuleInstance = Instance.new("ModuleScript") local wasFired = false @@ -145,12 +147,14 @@ describe("loadedModuleChanged", function() -- Require the module so that events get setup loader:require(mockModuleInstance) + expect(wasFired).toBe(false) + mockModuleInstance.Source = "Something different" expect(wasFired).toBe(true) end) - it("should fire for every consumer up the chain", function() + test("fires for every consumer up the chain", function() tree = createModuleTest({ ModuleA = [[ return "ModuleA" @@ -183,7 +187,7 @@ describe("loadedModuleChanged", function() end) describe("cache", function() - it("should add a module and its result to the cache", function() + test("adds a module and its result to the cache", function() local mockModuleInstance = Instance.new("ModuleScript") loader:cache(mockModuleInstance, mockModuleSource) @@ -196,14 +200,14 @@ describe("cache", function() end) describe("require", function() - it("should add the module to the cache", function() + test("adds the module to the cache", function() local mockModuleInstance = Instance.new("ModuleScript") loader:require(mockModuleInstance) expect(loader._cache[mockModuleInstance:GetFullName()]).toBeDefined() end) - it("should return cached results", function() + test("returns cached results", function() tree = createModuleTest({ -- We return a table since it can act as a unique symbol. So if -- both consumers are getting the same table we can perform an @@ -228,7 +232,7 @@ describe("require", function() expect(sharedModuleFromConsumer1).toBe(sharedModuleFromConsumer2) end) - it("should add the calling script as a consumer", function() + test("adds the calling script as a consumer", function() tree = createModuleTest({ SharedModule = [[ local module = {} @@ -248,7 +252,7 @@ describe("require", function() expect(cachedModule.consumers[tree.Consumer:GetFullName()]).toBeDefined() end) - it("should update consumers when requiring a cached module from a different script", function() + test("updates consumers when requiring a cached module from a different script", function() tree = createModuleTest({ SharedModule = [[ local module = {} @@ -277,7 +281,7 @@ describe("require", function() expect(cachedModule.consumers[tree.Consumer2:GetFullName()]).toBeDefined() end) - it("should keep track of _G between modules", function() + test("keeps track of _G between modules", function() tree = createModuleTest({ WriteGlobal = [[ _G.foo = true @@ -297,7 +301,7 @@ describe("require", function() expect(result).toBe(true) end) - it("should keep track of _G in nested requires", function() + test("keeps track of _G in nested requires", function() tree = createModuleTest({ DefineGlobal = [[ _G.foo = true @@ -318,7 +322,7 @@ describe("require", function() expect(loader._globals.foo).toBeUndefined() end) - it("should add globals on _G to the cachedModule's globals", function() + test("adds globals on _G to the cachedModule's globals", function() tree = createModuleTest({ DefineGlobal = [[ _G.foo = true @@ -334,7 +338,7 @@ describe("require", function() end) describe("clearModule", function() - it("should clear a module from the cache", function() + test("clears a module from the cache", function() tree = createModuleTest({ Module = [[ return "Module" @@ -350,7 +354,7 @@ describe("clearModule", function() expect(loader._cache[tree.Module:GetFullName()]).toBeUndefined() end) - it("should clear all consumers of a module from the cache", function() + test("clears all consumers of a module from the cache", function() tree = createModuleTest({ SharedModule = [[ local module = {} @@ -380,7 +384,7 @@ describe("clearModule", function() expect(loader._cache[tree.SharedModule:GetFullName()]).toBeUndefined() end) - it("should only clear modules in the consumer chain", function() + test("only clears modules in the consumer chain", function() tree = createModuleTest({ Module = [[ return nil @@ -405,7 +409,7 @@ describe("clearModule", function() expect(loader._cache[tree.Independent:GetFullName()]).toBeDefined() end) - it("should clear all globals that a module supplied", function() + test("clears all globals that a module supplied", function() tree = createModuleTest({ DefineGlobalFoo = [[ _G.foo = true @@ -426,7 +430,7 @@ describe("clearModule", function() expect(loader._globals.bar).toBeUndefined() end) - it("should fire loadedModuleChanged when clearing a module", function() + test("fires loadedModuleChanged when clearing a module", function() tree = createModuleTest({ Module = [[ return nil @@ -449,7 +453,7 @@ describe("clearModule", function() expect(wasFired).toBe(true) end) - it("should fire loadedModuleChanged for every module up the chain", function() + test("fires loadedModuleChanged for every module up the chain", function() tree = createModuleTest({ Module3 = [[ return {} @@ -480,7 +484,7 @@ describe("clearModule", function() expect(count).toBe(4) end) - it("should not fire loadedModuleChanged for a module that hasn't been required", function() + test("never fires loadedModuleChanged for a module that hasn't been required", function() local wasFired = false loader.loadedModuleChanged:Connect(function() @@ -495,7 +499,7 @@ describe("clearModule", function() end) describe("clear", function() - it("should remove all modules from the cache", function() + test("removes all modules from the cache", function() local mockModuleInstance = Instance.new("ModuleScript") loader:cache(mockModuleInstance, mockModuleSource) @@ -507,7 +511,7 @@ describe("clear", function() expect(countDict(loader._cache)).toBe(0) end) - it("should reset globals", function() + test("reset globals", function() local globals = loader._globals loader:clear() @@ -534,7 +538,7 @@ describe("consumers", function() }) end) - it("should remove all consumers of a changed module from the cache", function() + test("removes all consumers of a changed module from the cache", function() loader:require(tree.ModuleA) local hasItems = next(loader._cache) ~= nil @@ -547,7 +551,7 @@ describe("consumers", function() expect(hasItems).toBe(false) end) - it("should not interfere with other cached modules", function() + test("does not interfere with other cached modules", function() loader:require(tree.ModuleA) loader:require(tree.ModuleC) @@ -591,10 +595,10 @@ describe("roblox-ts", function() rbxtsInclude:Destroy() end) - it("clearModule() should never clear the roblox-ts runtime from the cache", function() - -- This example isn't quite how a roblox-ts project would be setup - -- in practice since the require's for `Shared` would be using - -- `TS.import`, but it should be close enough for our test case + test("clearModule() never clears the roblox-ts runtime from the cache", function() + -- This example isn't quite how a roblox-ts project would be setup since + -- the requires for `Shared` would be using `TS.import`, but it should + -- be close enough for our test case tree = createModuleTest({ Shared = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) @@ -627,7 +631,7 @@ describe("roblox-ts", function() expect(loader._cache[tree.Root:GetFullName()]).toBeUndefined() end) - it("clear() should clear the roblox-ts runtime when calling", function() + test("clear() clears the roblox-ts runtime when calling", function() tree = createModuleTest({ Module = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) diff --git a/src/bind.spec.luau b/src/bind.spec.luau index 570faff..9817b0b 100644 --- a/src/bind.spec.luau +++ b/src/bind.spec.luau @@ -1,10 +1,10 @@ local JestGlobals = require("@pkg/JestGlobals") -local it = JestGlobals.it +local test = JestGlobals.test local expect = JestGlobals.expect local bind = require("./bind") -it("should bind 'self' to the given callback", function() +test("binds 'self' to the given callback", function() local module = { value = "foo", callback = function(self) @@ -17,7 +17,7 @@ it("should bind 'self' to the given callback", function() expect(callback()).toBe("foo") end) -it("should work for the usage example", function() +test("works for the usage example", function() local Class = {} Class.__index = Class diff --git a/src/createTablePassthrough.spec.luau b/src/createTablePassthrough.spec.luau index b11caa9..f8191ff 100644 --- a/src/createTablePassthrough.spec.luau +++ b/src/createTablePassthrough.spec.luau @@ -1,10 +1,10 @@ local JestGlobals = require("@pkg/JestGlobals") -local it = JestGlobals.it +local test = JestGlobals.test local expect = JestGlobals.expect local createTablePassthrough = require("./createTablePassthrough") -it("should work for the use case of maintaining global variables", function() +test("works for the use case of maintaining global variables", function() local allGlobals = {} local moduleGlobals1 = createTablePassthrough(allGlobals) local moduleGlobals2 = createTablePassthrough(allGlobals) diff --git a/src/getEnv.spec.luau b/src/getEnv.spec.luau index 425b9b5..ee8948e 100644 --- a/src/getEnv.spec.luau +++ b/src/getEnv.spec.luau @@ -1,19 +1,19 @@ local JestGlobals = require("@pkg/JestGlobals") -local it = JestGlobals.it +local test = JestGlobals.test local expect = JestGlobals.expect local getEnv = require("./getEnv") -it("should return a table", function() +test("returns a table", function() expect(typeof(getEnv())).toBe("table") end) -it("should have the correct 'script' global", function() +test("has the correct 'script' global", function() local env = getEnv(script.Parent.getEnv) expect(env.script).toBe(script.Parent.getEnv) end) -it("should set _G to the 'globals' argument", function() +test("sets _G to the 'globals' argument", function() local globals = {} local env = getEnv(script.Parent.getEnv, globals) diff --git a/src/getRobloxTsRuntime.spec.luau b/src/getRobloxTsRuntime.spec.luau index b5ffe96..2509cf9 100644 --- a/src/getRobloxTsRuntime.spec.luau +++ b/src/getRobloxTsRuntime.spec.luau @@ -1,12 +1,12 @@ local ReplicatedStorage = game:GetService("ReplicatedStorage") local JestGlobals = require("@pkg/JestGlobals") -local it = JestGlobals.it +local test = JestGlobals.test local expect = JestGlobals.expect local getRobloxTsRuntime = require("./getRobloxTsRuntime") -it("should retrieve the roblox-ts runtime library", function() +test("retrieves the roblox-ts runtime library", function() local includes = Instance.new("Folder") includes.Name = "rbxts_include" includes.Parent = ReplicatedStorage @@ -22,7 +22,7 @@ it("should retrieve the roblox-ts runtime library", function() expect(runtime == mockRuntime).toBe(true) end) -it("should return nil if the runtime can't be found", function() +test("returns nil if the runtime can't be found", function() local runtime = getRobloxTsRuntime() expect(runtime).toBeUndefined() end) From f9205fb1ba17ba3577f179ee30c64f2f0f220473 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 15:27:32 -0800 Subject: [PATCH 04/28] Remove _getSource test since it doesn't provide much value --- src/ModuleLoader.spec.luau | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index c53df5e..89e7e3b 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -61,28 +61,6 @@ afterEach(function() end end) -describe("_getSource", function() - -- This test doesn't supply much value. Essentially, the "Source" - -- property requires elevated permissions, so we need the _getSource - -- method so that that if tests are being run from within a normal - -- script context that an error will not be produced. - it("should return the Source property if it can be indexed", function() - local mockModuleInstance = Instance.new("ModuleScript") - - local canIndex = pcall(function() - return mockModuleInstance.Source - end) - - local source = loader:_getSource(mockModuleInstance) - - if canIndex then - expect(source).toBeDefined() - else - expect(source).toBeUndefined() - end - end) -end) - describe("_trackChanges", function() test("creates a Janitor instance if it doesn't exist", function() local mockModuleInstance = Instance.new("ModuleScript") From 3c80d9062c139b8f74ffa5758efbed4d97f9ed3c Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 16 Dec 2024 15:29:02 -0800 Subject: [PATCH 05/28] Remove loadmodule support --- src/ModuleLoader.luau | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index d3b80df..719e7d3 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -6,9 +6,6 @@ local getCallerPath = require("./getCallerPath") local getEnv = require("./getEnv") local getRobloxTsRuntime = require("./getRobloxTsRuntime") -local loadmodule = (debug :: any).loadmodule -local loadmoduleEnabled = pcall(loadmodule, Instance.new("ModuleScript")) - type CachedModuleResult = any type ModuleConsumers = { @@ -225,13 +222,7 @@ function ModuleLoader:require(module: ModuleScript) end local source = self:_getSource(module) - local moduleFn, parseError - - if loadmoduleEnabled then - moduleFn, parseError = loadmodule(module) - else - moduleFn, parseError = self._loadstring(source, module:GetFullName()) - end + local moduleFn, parseError = self._loadstring(source, module:GetFullName()) if not moduleFn then local message = if parseError then parseError else "" From 9e4123bb2bb8c806577553ec6cb5033e203ff8fe Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 13:31:40 -0800 Subject: [PATCH 06/28] Cleanup the loadstring stacktrace --- src/ModuleLoader.luau | 21 +++++++++---------- src/ModuleLoader.spec.luau | 38 +++++++++++++++++++++++++++++++++++ src/cleanLoadstringStack.luau | 19 ++++++++++++++++++ 3 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 src/cleanLoadstringStack.luau diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 719e7d3..26f78d8 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -1,6 +1,7 @@ local Janitor = require("@pkg/Janitor") local bind = require("./bind") +local cleanLoadstringStack = require("./cleanLoadstringStack") local createTablePassthrough = require("./createTablePassthrough") local getCallerPath = require("./getCallerPath") local getEnv = require("./getEnv") @@ -113,13 +114,13 @@ end function ModuleLoader:_loadCachedModule(module: ModuleScript) local cachedModule: CachedModule = self._cache[module:GetFullName()] - assert( - cachedModule.isLoaded, - "Requested module experienced an error while loading MODULE: " - .. module:GetFullName() - .. " - RESULT: " - .. tostring(cachedModule.result) - ) + if not cachedModule.isLoaded then + error( + `requested module experienced an error while loading` + .. `\n- MODULE: {module:GetFullName()}` + .. `\n- RESULT: {cachedModule.result}` + ) + end return cachedModule.result end @@ -225,8 +226,7 @@ function ModuleLoader:require(module: ModuleScript) local moduleFn, parseError = self._loadstring(source, module:GetFullName()) if not moduleFn then - local message = if parseError then parseError else "" - error(`Could not parse {module:GetFullName()}: {message}`) + error(if parseError then cleanLoadstringStack(parseError) else "") end local globals = createTablePassthrough(self._globals) @@ -247,12 +247,11 @@ function ModuleLoader:require(module: ModuleScript) setfenv(moduleFn, env) local success, result = xpcall(moduleFn, debug.traceback) - if success then newCachedModule.isLoaded = true newCachedModule.result = result else - error(("Error requiring %s: %s"):format(module.Name, result)) + error(result) end self:_trackChanges(module) diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index 89e7e3b..e03d376 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -313,6 +313,44 @@ describe("require", function() local cachedModule = loader._cache[tree.DefineGlobal:GetFullName()] expect(cachedModule.globals.foo).toBe(true) end) + + test("handles syntax errors for direct require", function() + tree = createModuleTest({ + Module = [[ + syntax error + ]], + }) + tree.Name = "SyntaxError" + + expect(function() + loader:require(tree.Module) + end).toThrow(`SyntaxError.Module:1: Incomplete statement: expected assignment or a function call`) + end) + + test("handles syntax errors for nested requires", function() + tree = createModuleTest({ + Module3 = [[ + syntax error + ]], + Module2 = [[ + require(script.Parent.Module3) + return {} + ]], + Module1 = [[ + require(script.Parent.Module2) + return {} + ]], + Consumer = [[ + require(script.Parent.Module1) + return nil + ]], + }) + tree.Name = "SyntaxError" + + expect(function() + loader:require(tree.Consumer) + end).toThrow(`SyntaxError.Consumer:1: Incomplete statement: expected assignment or a function call`) + end) end) describe("clearModule", function() diff --git a/src/cleanLoadstringStack.luau b/src/cleanLoadstringStack.luau new file mode 100644 index 0000000..94b3b43 --- /dev/null +++ b/src/cleanLoadstringStack.luau @@ -0,0 +1,19 @@ +-- upstream: https://github.com/Roblox/jest-roblox/blob/408eac1b8d210e6e07387fb341fa9b9e181de897/src/roblox-shared/src/cleanLoadStringStack.lua + +return function(line: string): string + local spacing, filePath, lineNumber, extra = line:match('(%s*)%[string "(.-)"%]:(%d+)(.*)') + if filePath then + local match = filePath + if spacing then + match = spacing .. match + end + if lineNumber then + match = match .. ":" .. lineNumber + end + if extra then + match = match .. extra + end + return match + end + return line +end From 9cdfec98c2117c974941b83a399a334bee0b3e6d Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 13:52:23 -0800 Subject: [PATCH 07/28] Fix ancestry changes not clearing modules AncestryChanged fires after the module has already been moved so its current path is invalid for the cache. We're now keying by ModuleScript instances to better handle this case --- src/ModuleLoader.luau | 49 +++++++-------- src/getInstanceFromFullName.luau | 89 +++++++++++++++++++++++++++ src/getInstanceFromFullName.spec.luau | 75 ++++++++++++++++++++++ wally.toml | 1 + 4 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 src/getInstanceFromFullName.luau create mode 100644 src/getInstanceFromFullName.spec.luau diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 26f78d8..c5b2a67 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -5,12 +5,13 @@ local cleanLoadstringStack = require("./cleanLoadstringStack") local createTablePassthrough = require("./createTablePassthrough") local getCallerPath = require("./getCallerPath") local getEnv = require("./getEnv") +local getInstanceFromFullName = require("./getInstanceFromFullName") local getRobloxTsRuntime = require("./getRobloxTsRuntime") type CachedModuleResult = any type ModuleConsumers = { - [string]: boolean, + [LuaSourceContainer]: boolean, } -- Each module gets its own global table that it can modify via _G. This makes @@ -30,10 +31,10 @@ type CachedModule = { } type ModuleLoaderProps = { - _cache: { [string]: CachedModule }, + _cache: { [LuaSourceContainer]: CachedModule }, _loadstring: typeof(loadstring), _debugInfo: typeof(debug.info), - _janitors: { [string]: any }, + _janitors: { [LuaSourceContainer]: any }, _globals: { [any]: any }, _loadedModuleChangedBindable: BindableEvent, @@ -112,7 +113,7 @@ function ModuleLoader.new() end function ModuleLoader:_loadCachedModule(module: ModuleScript) - local cachedModule: CachedModule = self._cache[module:GetFullName()] + local cachedModule: CachedModule = self._cache[module] if not cachedModule.isLoaded then error( @@ -152,7 +153,7 @@ end @private ]=] function ModuleLoader:_trackChanges(module: ModuleScript) - local existingJanitor = self._janitors[module:GetFullName()] + local existingJanitor = self._janitors[module] local janitor = if existingJanitor then existingJanitor else Janitor.new() janitor:Cleanup() @@ -167,7 +168,7 @@ function ModuleLoader:_trackChanges(module: ModuleScript) end end)) - self._janitors[module:GetFullName()] = janitor + self._janitors[module] = janitor end --[=[ @@ -194,7 +195,7 @@ function ModuleLoader:cache(module: ModuleScript, result: any) globals = createTablePassthrough(self._globals), } - self._cache[module:GetFullName()] = cachedModule + self._cache[module] = cachedModule end --[=[ @@ -210,15 +211,15 @@ end ``` ]=] function ModuleLoader:require(module: ModuleScript) - local cachedModule = self._cache[module:GetFullName()] + local cachedModule = self._cache[module] local callerPath = getCallerPath() - - if not callerPath then - return nil - end + local caller = if callerPath then getInstanceFromFullName(callerPath) else nil if cachedModule then - cachedModule.consumers[callerPath] = true + if caller and caller:IsA("LuaSourceContainer") then + cachedModule.consumers[caller] = true + end + return self:_loadCachedModule(module) end @@ -235,12 +236,13 @@ function ModuleLoader:require(module: ModuleScript) module = module, result = nil, isLoaded = false, - consumers = { - [callerPath] = true, - }, + consumers = {}, globals = globals, } - self._cache[module:GetFullName()] = newCachedModule + if caller and caller:IsA("LuaSourceContainer") then + newCachedModule.consumers[caller] = true + end + self._cache[module] = newCachedModule local env: any = getEnv(module, globals) env.require = bind(self, self.require) @@ -273,7 +275,7 @@ function ModuleLoader:_getConsumers(module: ModuleScript): { ModuleScript } end end - local cachedModule: CachedModule = self._cache[module:GetFullName()] + local cachedModule: CachedModule = self._cache[module] local found = {} getConsumersRecursively(cachedModule, found) @@ -286,8 +288,9 @@ function ModuleLoader:_getConsumers(module: ModuleScript): { ModuleScript } return consumers end +-- function ModuleLoader:clearModule(moduleToClear: ModuleScript) - if not self._cache[moduleToClear:GetFullName()] then + if not self._cache[moduleToClear] then return end @@ -300,18 +303,16 @@ function ModuleLoader:clearModule(moduleToClear: ModuleScript) end for _, module in modulesToClear do - local fullName = module:GetFullName() - - local cachedModule = self._cache[fullName] + local cachedModule = self._cache[module] if cachedModule then - self._cache[fullName] = nil + self._cache[module] = nil for key in cachedModule.globals do self._globals[key] = nil end - local janitor = self._janitors[fullName] + local janitor = self._janitors[module] janitor:Cleanup() end end diff --git a/src/getInstanceFromFullName.luau b/src/getInstanceFromFullName.luau new file mode 100644 index 0000000..1486fe3 --- /dev/null +++ b/src/getInstanceFromFullName.luau @@ -0,0 +1,89 @@ +--[[ + Gets an instance based off the result of GetFullName(). + + This is used in conjunction with debug.info() to locate the calling script. + + Returns nil if the instance is outside the DataModel. +]] + +local Sift = require("@pkg/Sift") + +local PATH_SEPERATOR = "." + +local function canAccess(instance: Instance): boolean + local success = pcall(function() + return instance.Name + end) + + return success +end + +local function maybeGetService(serviceName: string): Instance? + local success, current: any = pcall(function() + return game:GetService(serviceName) + end) + + if success and current and canAccess(current) and current:IsA("Instance") then + return current + else + return nil + end +end + +local function getInstanceFromFullName(fullName: string): Instance? + local parts = fullName:split(PATH_SEPERATOR) + local serviceName = table.remove(parts, 1) + + if serviceName then + -- This function only works for instances in the DataModel. As such, the + -- first part of the path will always be a service, so if we can't find + -- one we exit out and return nil + local current = maybeGetService(serviceName) + + if current then + -- TODO: Verify this isn't also needed with the below TODO + -- if #parts == 1 then + -- return current + -- end + + while #parts > 0 do + -- Keep around a copy of the `parts` array. We are going to concat this + -- into new paths, and incrementally remove from the right to narrow + -- down the file path. + local tempParts = Sift.Array.copy(parts) + + -- The result of GetFullName() uses dots to separate paths, but we also + -- use dots in our file names (e.g. with spec and story files). As such, + -- this block will look ahead to see if multiple parts are actually a + -- single filename. + for _ = 1, #tempParts do + local name = table.concat(tempParts, PATH_SEPERATOR) + local found = current:FindFirstChild(name) + + if found then + current = found + parts = Sift.List.shift(parts, #name:split(PATH_SEPERATOR)) + break + else + if #tempParts == 1 then + -- TODO: Need this early exit to handle cases with instances not existing later on. Verify + -- if this can be effectively repro'd by calling this function on an instance that doesn't + -- exist. Particularly make sure to test look-aheads for `Foo.story`, it seemed like the + -- loop got stuck looking for "story" when "ChromeWindow.story" didn't exist + return nil + else + -- Reduce from the right until we find the next instance + tempParts = Sift.List.pop(tempParts) + end + end + end + end + + return current + end + end + + return nil +end + +return getInstanceFromFullName diff --git a/src/getInstanceFromFullName.spec.luau b/src/getInstanceFromFullName.spec.luau new file mode 100644 index 0000000..930fa8d --- /dev/null +++ b/src/getInstanceFromFullName.spec.luau @@ -0,0 +1,75 @@ +local ReplicatedStorage = game:GetService("ReplicatedStorage") + +local JestGlobals = require("@pkg/JestGlobals") +local newFolder = require("@root/Testing/newFolder") + +local getInstanceFromFullName = require("./getInstanceFromFullName") + +local expect = JestGlobals.expect +local test = JestGlobals.test +local afterEach = JestGlobals.afterEach + +local folder: Folder + +afterEach(function() + if folder then + folder:Destroy() + end +end) + +test("gets services", function() + local path = getInstanceFromFullName("ReplicatedStorage") + expect(path).toBe(ReplicatedStorage) +end) + +test("works on nested instances", function() + local module = Instance.new("ModuleScript") + + folder = newFolder({ + foo = newFolder({ + bar = module, + }), + }) + folder.Parent = ReplicatedStorage + + local path = getInstanceFromFullName(module:GetFullName()) + expect(path).toBe(module) +end) + +test("works with spec files", function() + local module = Instance.new("ModuleScript") + + folder = newFolder({ + foo = newFolder({ + ["bar.spec"] = module, + }), + }) + folder.Parent = ReplicatedStorage + + local path = getInstanceFromFullName(module:GetFullName()) + expect(path).toBe(module) +end) + +test("finds spec files BEFORE the module it is associated with", function() + local module = Instance.new("ModuleScript") + + folder = newFolder({ + foo = newFolder({ + bar = Instance.new("ModuleScript"), + ["bar.spec"] = module, + }), + }) + folder.Parent = ReplicatedStorage + + local path = getInstanceFromFullName(module:GetFullName()) + expect(path).toBe(module) +end) + +test("returns nil if the first part of the path is not a service", function() + expect(getInstanceFromFullName("Part")).toBeUndefined() +end) + +test("returns nil when instance with an extension does not exist", function() + expect(getInstanceFromFullName("foo.story")).toBeUndefined() + expect(getInstanceFromFullName("Path.To.Foo.story")).toBeUndefined() +end) diff --git a/wally.toml b/wally.toml index f38303b..df51e35 100644 --- a/wally.toml +++ b/wally.toml @@ -17,6 +17,7 @@ include = [ [dependencies] Janitor = "howmanysmall/janitor@1.13.15" +Sift = "csqrl/sift@0.0.8" #[dev-dependencies] Jest = "jsdotlua/jest@3.6.1-rc.2" From 3577403370b1d9f2fa6dab00b5e9aa237645d22c Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 13:53:31 -0800 Subject: [PATCH 08/28] Fix build error --- src/Testing/newFolder.luau | 13 ++++++++++++ src/Testing/newFolder.spec.luau | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 src/Testing/newFolder.luau create mode 100644 src/Testing/newFolder.spec.luau diff --git a/src/Testing/newFolder.luau b/src/Testing/newFolder.luau new file mode 100644 index 0000000..2675b69 --- /dev/null +++ b/src/Testing/newFolder.luau @@ -0,0 +1,13 @@ +local function newFolder(children: { [string]: Instance }): Folder + local folder = Instance.new("Folder") + folder.Name = "Root" + + for name, child in pairs(children) do + child.Name = name + child.Parent = folder + end + + return folder +end + +return newFolder diff --git a/src/Testing/newFolder.spec.luau b/src/Testing/newFolder.spec.luau new file mode 100644 index 0000000..32ce1ba --- /dev/null +++ b/src/Testing/newFolder.spec.luau @@ -0,0 +1,36 @@ +local JestGlobals = require("@pkg/JestGlobals") +local newFolder = require("./newFolder") + +local expect = JestGlobals.expect +local test = JestGlobals.test + +test("return a folder named 'Root'", function() + local folder = newFolder({}) + expect(folder:IsA("Folder")).toBe(true) + expect(folder.Name).toBe("Root") +end) + +test("name children after the dictionary keys", function() + local child1 = Instance.new("Part") + local child2 = Instance.new("Model") + + local folder: any = newFolder({ + Child1 = child1, + Child2 = child2, + }) + + expect(folder.Child1).toBe(child1) + expect(folder.Child2).toBe(child2) +end) + +test("support nesting newFolder as children", function() + local folder: any = newFolder({ + Child = newFolder({ + AnotherChild = newFolder({ + Module = Instance.new("ModuleScript"), + }), + }), + }) + + expect(folder.Child.AnotherChild.Module).toBeDefined() +end) From ea9d979226ea6982b78170d463a19e0cffebd6d2 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 15:48:01 -0800 Subject: [PATCH 09/28] Reference JestRuntime for module loading --- src/ModuleLoader.luau | 275 ++++++++++++++------------ src/ModuleLoader.spec.luau | 62 +++--- src/bind.luau | 35 ---- src/bind.spec.luau | 38 ---- src/createTablePassthrough.luau | 32 --- src/createTablePassthrough.spec.luau | 25 --- src/getCallerPath.luau | 32 --- src/getEnv.luau | 31 --- src/getEnv.spec.luau | 24 --- src/getInstanceFromFullName.luau | 89 --------- src/getInstanceFromFullName.spec.luau | 75 ------- 11 files changed, 182 insertions(+), 536 deletions(-) delete mode 100644 src/bind.luau delete mode 100644 src/bind.spec.luau delete mode 100644 src/createTablePassthrough.luau delete mode 100644 src/createTablePassthrough.spec.luau delete mode 100644 src/getCallerPath.luau delete mode 100644 src/getEnv.luau delete mode 100644 src/getEnv.spec.luau delete mode 100644 src/getInstanceFromFullName.luau delete mode 100644 src/getInstanceFromFullName.spec.luau diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index c5b2a67..d207c67 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -1,41 +1,35 @@ local Janitor = require("@pkg/Janitor") -local bind = require("./bind") local cleanLoadstringStack = require("./cleanLoadstringStack") -local createTablePassthrough = require("./createTablePassthrough") -local getCallerPath = require("./getCallerPath") -local getEnv = require("./getEnv") -local getInstanceFromFullName = require("./getInstanceFromFullName") local getRobloxTsRuntime = require("./getRobloxTsRuntime") -type CachedModuleResult = any +local loadmodule = (debug :: any).loadmodule +local loadModuleEnabled = pcall(function() + return loadmodule(Instance.new("ModuleScript")) +end) -type ModuleConsumers = { - [LuaSourceContainer]: boolean, -} +local function getModuleSource(moduleScript: ModuleScript): string + local success, result = pcall(function() + return moduleScript.Source + end) --- Each module gets its own global table that it can modify via _G. This makes --- it easy to clear out a module and the globals it defines without impacting --- other modules. A module's function environment has all globals merged --- together on _G -type ModuleGlobals = { - [any]: any, -} + return if success then result else "" +end -type CachedModule = { - module: ModuleScript, +type LoadedModuleExports = unknown + +type LoadedModule = { + instance: ModuleScript, isLoaded: boolean, - result: CachedModuleResult, - consumers: ModuleConsumers, - globals: ModuleGlobals, + exports: LoadedModuleExports, + children: { [ModuleScript]: boolean }, } type ModuleLoaderProps = { - _cache: { [LuaSourceContainer]: CachedModule }, + _moduleRegistry: { [ModuleScript]: LoadedModule }, + _loadedModuleFns: { [ModuleScript]: { any } }, _loadstring: typeof(loadstring), - _debugInfo: typeof(debug.info), - _janitors: { [LuaSourceContainer]: any }, - _globals: { [any]: any }, + _janitors: { [ModuleScript]: any }, _loadedModuleChangedBindable: BindableEvent, loadedModuleChanged: RBXScriptSignal, @@ -51,8 +45,8 @@ type ModuleLoaderImpl = { clearModule: (self: ModuleLoader, module: ModuleScript) -> (), clear: (self: ModuleLoader) -> (), - _loadCachedModule: (self: ModuleLoader, module: ModuleScript) -> CachedModuleResult, - _getSource: (self: ModuleLoader, module: ModuleScript) -> string, + _loadModule: (self: ModuleLoader, loadedModule: LoadedModule) -> (), + _execModule: (self: ModuleLoader, loadedModule: LoadedModule) -> (), _trackChanges: (self: ModuleLoader, module: ModuleScript) -> (), _getConsumers: (self: ModuleLoader, module: ModuleScript) -> { ModuleScript }, } @@ -79,11 +73,10 @@ ModuleLoader.__index = ModuleLoader function ModuleLoader.new() local self = {} - self._cache = {} + self._moduleRegistry = {} + self._loadedModuleFns = {} self._loadstring = loadstring - self._debugInfo = debug.info self._janitors = {} - self._globals = {} self._loadedModuleChangedBindable = Instance.new("BindableEvent") --[=[ @@ -112,37 +105,6 @@ function ModuleLoader.new() return setmetatable(self, ModuleLoader) end -function ModuleLoader:_loadCachedModule(module: ModuleScript) - local cachedModule: CachedModule = self._cache[module] - - if not cachedModule.isLoaded then - error( - `requested module experienced an error while loading` - .. `\n- MODULE: {module:GetFullName()}` - .. `\n- RESULT: {cachedModule.result}` - ) - end - - return cachedModule.result -end - ---[=[ - Gets the Source of a ModuleScript. - - This method exists primarily so we can better write unit tests. Attempting - to index the Source property from a regular script context throws an error, - so this method allows us to safely fallback in tests. - - @private -]=] -function ModuleLoader:_getSource(module: ModuleScript): string - local success, result = pcall(function() - return module.Source - end) - - return if success then result else "" -end - --[=[ Tracks the changes to a required module's ancestry and `Source`. @@ -187,15 +149,102 @@ end ``` ]=] function ModuleLoader:cache(module: ModuleScript, result: any) - local cachedModule: CachedModule = { - module = module, - result = result, + local loadedModule: LoadedModule = { + instance = module, + exports = result, isLoaded = true, - consumers = {}, - globals = createTablePassthrough(self._globals), + children = {}, } - self._cache[module] = cachedModule + self._moduleRegistry[module] = loadedModule +end + +function ModuleLoader:_execModule(localModule: LoadedModule) + local moduleFunction, defaultEnvironment, errorMessage, cleanupFn + local module = localModule.instance + + local loadedModule = self._loadedModuleFns[module] + if loadedModule then + moduleFunction = loadedModule[1] + defaultEnvironment = loadedModule[2] + else + -- Narrowing this type here lets us appease the type checker while still + -- counting on types for the rest of this file + if loadModuleEnabled then + moduleFunction, errorMessage, cleanupFn = loadmodule(module) + else + moduleFunction = loadstring(getModuleSource(module), module:GetFullName()) + end + + assert(moduleFunction, errorMessage) + + -- Cache initial environment table to inherit from later + defaultEnvironment = getfenv(moduleFunction) + + self._loadedModuleFns[module] = { moduleFunction, defaultEnvironment, cleanupFn } + end + + -- The default behavior for function environments is to inherit the table + -- instance from the parent environment. This means that each invocation of + -- `moduleFunction()` will return a new module instance but with the same + -- environment table as `moduleFunction` itself at the time of invocation. + -- In order to properly sanbox module instances, we need to ensure that each + -- instance has its own distinct environment table containing the specific + -- overrides for it, but still inherits from the default parent environment + -- for non-overriden environment goodies. + + -- This is the 'least mocked' environment that scripts will be able to see. + -- The final function environment inherits from this sandbox. This is + -- separate so that, in the future, `globalEnv` could expose these + -- 'unmocked' functions instead of the ones in the global environment. + local sandboxEnvironment = setmetatable({ + script = if loadModuleEnabled then defaultEnvironment.script else module, + game = defaultEnvironment.game, + workspace = defaultEnvironment.workspace, + plugin = defaultEnvironment.plugin, + + -- legacy aliases for data model + Game = defaultEnvironment.game, + Workspace = defaultEnvironment.workspace, + + require = function(otherModule: ModuleScript | string) + if typeof(otherModule) == "string" then + -- Disabling this at the surface level of the API until we have + -- deeper support in Jest. + error("Require-by-string is not enabled for use inside Jest at this time.") + end + return self:require(otherModule) + end, + }, { + __index = defaultEnvironment, + }) + + -- This is the environment actually passed to scripts, including all global + -- mocks and other customisations the user might choose to apply. + local mockedSandboxEnvironment = setmetatable({}, { + __index = sandboxEnvironment, + }) + + setfenv(moduleFunction, mockedSandboxEnvironment :: any) + local moduleResult = table.pack(moduleFunction()) + + if moduleResult.n ~= 1 then + error( + string.format( + "[Module Error]: %s did not return a valid result\n" .. "\tModuleScripts must return exactly one value", + tostring(module) + ) + ) + end + + self:_trackChanges(module) + + localModule.exports = moduleResult[1] +end + +function ModuleLoader:_loadModule(localModule: LoadedModule) + self:_execModule(localModule) + localModule.isLoaded = true end --[=[ @@ -210,75 +259,57 @@ end local module = loader:require(script.Parent.ModuleScript) ``` ]=] -function ModuleLoader:require(module: ModuleScript) - local cachedModule = self._cache[module] - local callerPath = getCallerPath() - local caller = if callerPath then getInstanceFromFullName(callerPath) else nil - - if cachedModule then - if caller and caller:IsA("LuaSourceContainer") then - cachedModule.consumers[caller] = true - end - - return self:_loadCachedModule(module) +function ModuleLoader:require(module: ModuleScript): unknown + if module.Name:find(".global$") then + return (require :: any)(module) end - local source = self:_getSource(module) - local moduleFn, parseError = self._loadstring(source, module:GetFullName()) - - if not moduleFn then - error(if parseError then cleanLoadstringStack(parseError) else "") + local existingModule = self._moduleRegistry[module] + if existingModule then + return existingModule.exports end - local globals = createTablePassthrough(self._globals) - - local newCachedModule: CachedModule = { - module = module, - result = nil, + -- We must register the pre-allocated module object first so that any + -- circular dependencies that may arise while evaluating the module can + -- be satisfied. + local loadedModule: LoadedModule = { + instance = module, + exports = nil, isLoaded = false, - consumers = {}, - globals = globals, + children = {}, } - if caller and caller:IsA("LuaSourceContainer") then - newCachedModule.consumers[caller] = true - end - self._cache[module] = newCachedModule - local env: any = getEnv(module, globals) - env.require = bind(self, self.require) - setfenv(moduleFn, env) + self._moduleRegistry[module] = loadedModule - local success, result = xpcall(moduleFn, debug.traceback) - if success then - newCachedModule.isLoaded = true - newCachedModule.result = result - else + local success, result = pcall(function() + self:_loadModule(loadedModule) + end) + if not success then + self._moduleRegistry[module] = nil error(result) end - self:_trackChanges(module) - - return self:_loadCachedModule(module) + return loadedModule.exports end function ModuleLoader:_getConsumers(module: ModuleScript): { ModuleScript } - local function getConsumersRecursively(cachedModule: CachedModule, found: { [ModuleScript]: true }) - for consumer in cachedModule.consumers do - local cachedConsumer = self._cache[consumer] - - if cachedConsumer then - if not found[cachedConsumer.module] then - found[cachedConsumer.module] = true - getConsumersRecursively(cachedConsumer, found) + local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) + for consumer in loadedModule.children do + local loadedChildModule = self._moduleRegistry[consumer] + + if loadedChildModule then + if not found[loadedChildModule.instance] then + found[loadedChildModule.instance] = true + getConsumersRecursively(loadedModule, found) end end end end - local cachedModule: CachedModule = self._cache[module] + local loadedModule: LoadedModule = self._moduleRegistry[module] local found = {} - getConsumersRecursively(cachedModule, found) + getConsumersRecursively(loadedModule, found) local consumers = {} for consumer in found do @@ -290,7 +321,7 @@ end -- function ModuleLoader:clearModule(moduleToClear: ModuleScript) - if not self._cache[moduleToClear] then + if not self._moduleRegistry[moduleToClear] then return end @@ -303,14 +334,10 @@ function ModuleLoader:clearModule(moduleToClear: ModuleScript) end for _, module in modulesToClear do - local cachedModule = self._cache[module] - - if cachedModule then - self._cache[module] = nil + local loadedModule = self._moduleRegistry[module] - for key in cachedModule.globals do - self._globals[key] = nil - end + if loadedModule then + self._moduleRegistry[module] = nil local janitor = self._janitors[module] janitor:Cleanup() @@ -344,7 +371,7 @@ end ``` ]=] function ModuleLoader:clear() - self._cache = {} + self._moduleRegistry = {} self._globals = {} for _, janitor in self._janitors do diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index e03d376..d43f902 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -170,7 +170,7 @@ describe("cache", function() loader:cache(mockModuleInstance, mockModuleSource) - local cachedModule = loader._cache[mockModuleInstance:GetFullName()] + local cachedModule = loader._moduleRegistry[mockModuleInstance:GetFullName()] expect(cachedModule).toBeDefined() expect(cachedModule.result).toBe(mockModuleSource) @@ -182,7 +182,7 @@ describe("require", function() local mockModuleInstance = Instance.new("ModuleScript") loader:require(mockModuleInstance) - expect(loader._cache[mockModuleInstance:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[mockModuleInstance:GetFullName()]).toBeDefined() end) test("returns cached results", function() @@ -224,7 +224,7 @@ describe("require", function() loader:require(tree.Consumer) - local cachedModule = loader._cache[tree.SharedModule:GetFullName()] + local cachedModule = loader._moduleRegistry[tree.SharedModule:GetFullName()] expect(cachedModule).toBeDefined() expect(cachedModule.consumers[tree.Consumer:GetFullName()]).toBeDefined() @@ -248,7 +248,7 @@ describe("require", function() loader:require(tree.Consumer1) - local cachedModule = loader._cache[tree.SharedModule:GetFullName()] + local cachedModule = loader._moduleRegistry[tree.SharedModule:GetFullName()] expect(cachedModule.consumers[tree.Consumer1:GetFullName()]).toBeDefined() expect(cachedModule.consumers[tree.Consumer2:GetFullName()]).toBeUndefined() @@ -310,7 +310,7 @@ describe("require", function() loader:require(tree.DefineGlobal) - local cachedModule = loader._cache[tree.DefineGlobal:GetFullName()] + local cachedModule = loader._moduleRegistry[tree.DefineGlobal:GetFullName()] expect(cachedModule.globals.foo).toBe(true) end) @@ -363,11 +363,11 @@ describe("clearModule", function() loader:require(tree.Module) - expect(loader._cache[tree.Module:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeDefined() loader:clearModule(tree.Module) - expect(loader._cache[tree.Module:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeUndefined() end) test("clears all consumers of a module from the cache", function() @@ -389,15 +389,15 @@ describe("clearModule", function() loader:require(tree.Consumer1) loader:require(tree.Consumer2) - expect(loader._cache[tree.Consumer1:GetFullName()]).toBeDefined() - expect(loader._cache[tree.Consumer2:GetFullName()]).toBeDefined() - expect(loader._cache[tree.SharedModule:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Consumer1:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Consumer2:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.SharedModule:GetFullName()]).toBeDefined() loader:clearModule(tree.SharedModule) - expect(loader._cache[tree.Consumer1:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.Consumer2:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.SharedModule:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Consumer1:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Consumer2:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.SharedModule:GetFullName()]).toBeUndefined() end) test("only clears modules in the consumer chain", function() @@ -417,12 +417,12 @@ describe("clearModule", function() loader:require(tree.Consumer) loader:require(tree.Independent) - expect(countDict(loader._cache)).toBe(3) + expect(countDict(loader._moduleRegistry)).toBe(3) loader:clearModule(tree.Module) - expect(countDict(loader._cache)).toBe(1) - expect(loader._cache[tree.Independent:GetFullName()]).toBeDefined() + expect(countDict(loader._moduleRegistry)).toBe(1) + expect(loader._moduleRegistry[tree.Independent:GetFullName()]).toBeDefined() end) test("clears all globals that a module supplied", function() @@ -520,11 +520,11 @@ describe("clear", function() loader:cache(mockModuleInstance, mockModuleSource) - expect(countDict(loader._cache)).toBe(1) + expect(countDict(loader._moduleRegistry)).toBe(1) loader:clear() - expect(countDict(loader._cache)).toBe(0) + expect(countDict(loader._moduleRegistry)).toBe(0) end) test("reset globals", function() @@ -557,13 +557,13 @@ describe("consumers", function() test("removes all consumers of a changed module from the cache", function() loader:require(tree.ModuleA) - local hasItems = next(loader._cache) ~= nil + local hasItems = next(loader._moduleRegistry) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - hasItems = next(loader._cache) ~= nil + hasItems = next(loader._moduleRegistry) ~= nil expect(hasItems).toBe(false) end) @@ -571,15 +571,15 @@ describe("consumers", function() loader:require(tree.ModuleA) loader:require(tree.ModuleC) - local hasItems = next(loader._cache) ~= nil + local hasItems = next(loader._moduleRegistry) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - expect(loader._cache[tree.ModuleA:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.ModuleB:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.ModuleC:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.ModuleA:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.ModuleB:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.ModuleC:GetFullName()]).toBeDefined() end) end) @@ -640,11 +640,11 @@ describe("roblox-ts", function() loader:require(tree.Root) loader:clearModule(tree.Shared) - expect(loader._cache[mockRuntime:GetFullName()]).toBeDefined() - expect(loader._cache[tree.Shared:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.Module1:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.Module2:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.Root:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[mockRuntime:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Shared:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module1:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module2:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Root:GetFullName()]).toBeUndefined() end) test("clear() clears the roblox-ts runtime when calling", function() @@ -657,7 +657,7 @@ describe("roblox-ts", function() loader:require(tree.Module) loader:clear() - expect(loader._cache[mockRuntime:GetFullName()]).toBeUndefined() - expect(loader._cache[tree.Module:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[mockRuntime:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeUndefined() end) end) diff --git a/src/bind.luau b/src/bind.luau deleted file mode 100644 index 27016a6..0000000 --- a/src/bind.luau +++ /dev/null @@ -1,35 +0,0 @@ ---[=[ - Binds an instance method so that it can be called like a function. - - Usage: - - ```lua - local Class = {} - Class.__index = Class - - function Class.new() - local self = {} - self.value = "foo" - return setmetatable(self, Class) - end - - function Class:getValue() - return self.value - end - - local instance = Class.new() - local getValue = bind(instance, instance.getValue) - - print(getValue()) -- "foo" - ``` - - @within ModuleLoader - @private -]=] -local function bind(self: T, callback: (self: T, ...any) -> any) - return function(...) - return callback(self, ...) - end -end - -return bind diff --git a/src/bind.spec.luau b/src/bind.spec.luau deleted file mode 100644 index 9817b0b..0000000 --- a/src/bind.spec.luau +++ /dev/null @@ -1,38 +0,0 @@ -local JestGlobals = require("@pkg/JestGlobals") -local test = JestGlobals.test -local expect = JestGlobals.expect - -local bind = require("./bind") - -test("binds 'self' to the given callback", function() - local module = { - value = "foo", - callback = function(self) - return self.value - end, - } - - local callback = bind(module, module.callback) - - expect(callback()).toBe("foo") -end) - -test("works for the usage example", function() - local Class = {} - Class.__index = Class - - function Class.new() - local self = {} - self.value = "foo" - return setmetatable(self, Class) - end - - function Class:getValue() - return self.value - end - - local instance = Class.new() - local getValue = bind(instance, instance.getValue) - - expect(getValue()).toBe("foo") -end) diff --git a/src/createTablePassthrough.luau b/src/createTablePassthrough.luau deleted file mode 100644 index cdb9258..0000000 --- a/src/createTablePassthrough.luau +++ /dev/null @@ -1,32 +0,0 @@ ---[[ - Creates a table that can be indexed and added to while also adding to a base - table. - - This is used for module globals so that a module can define variables on _G - which are maintained in a dictionary of all globals AND a dictionary of the - globals a given module has defined. - - This makes it easy to clear out the globals a modeule defines when removing - it from the cache. -]] - -type AnyTable = { [any]: any } - -local function createTablePassthrough(base: AnyTable): AnyTable - local proxy = {} - - setmetatable(proxy, { - __index = function(self, key) - local global = rawget(self, key) - return if global then global else base[key] - end, - __newindex = function(self, key, value) - base[key] = value - rawset(self, key, value) - end, - }) - - return proxy :: any -end - -return createTablePassthrough diff --git a/src/createTablePassthrough.spec.luau b/src/createTablePassthrough.spec.luau deleted file mode 100644 index f8191ff..0000000 --- a/src/createTablePassthrough.spec.luau +++ /dev/null @@ -1,25 +0,0 @@ -local JestGlobals = require("@pkg/JestGlobals") -local test = JestGlobals.test -local expect = JestGlobals.expect - -local createTablePassthrough = require("./createTablePassthrough") - -test("works for the use case of maintaining global variables", function() - local allGlobals = {} - local moduleGlobals1 = createTablePassthrough(allGlobals) - local moduleGlobals2 = createTablePassthrough(allGlobals) - - moduleGlobals1.foo = true - moduleGlobals2.bar = true - - expect(moduleGlobals1.foo).toBe(true) - expect(moduleGlobals1.bar).toBe(true) - expect(rawget(moduleGlobals1, "bar")).toBeUndefined() - - expect(moduleGlobals2.bar).toBe(true) - expect(moduleGlobals2.foo).toBe(true) - expect(rawget(moduleGlobals2, "foo")).toBeUndefined() - - expect(allGlobals.foo).toBe(true) - expect(allGlobals.bar).toBe(true) -end) diff --git a/src/getCallerPath.luau b/src/getCallerPath.luau deleted file mode 100644 index 921c3f5..0000000 --- a/src/getCallerPath.luau +++ /dev/null @@ -1,32 +0,0 @@ -local root = script.Parent - -local LOADSTRING_PATH_PATTERN = '%[string "(.*)"%]' - -local function getCallerPath(): string? - local level = 1 - - while true do - local path = debug.info(level, "s") - - if path then - -- Skip over any path that is a descendant of this package - if not path:find(root.Name, nil, true) then - -- Sometimes the path is represented as `[string "path.to.module"]` - -- so we match for the instance path and, if found, return it - local pathFromLoadstring = path:match(LOADSTRING_PATH_PATTERN) - - if pathFromLoadstring then - return pathFromLoadstring - else - return path - end - end - else - return nil - end - - level += 1 - end -end - -return getCallerPath diff --git a/src/getEnv.luau b/src/getEnv.luau deleted file mode 100644 index efb9871..0000000 --- a/src/getEnv.luau +++ /dev/null @@ -1,31 +0,0 @@ -local baseEnv = getfenv() - -local function getEnv(scriptRelativeTo: LuaSourceContainer?, globals: { [any]: any }?) - local newEnv = {} - - setmetatable(newEnv, { - __index = function(_, key) - if key ~= "plugin" then - return baseEnv[key] - else - return nil - end - end, - }) - - newEnv._G = globals - newEnv.script = scriptRelativeTo - - local realDebug = debug - - newEnv.debug = setmetatable({ - traceback = function(message) - -- Block traces to prevent overly verbose TestEZ output - return message or "" - end, - }, { __index = realDebug }) - - return newEnv -end - -return getEnv diff --git a/src/getEnv.spec.luau b/src/getEnv.spec.luau deleted file mode 100644 index ee8948e..0000000 --- a/src/getEnv.spec.luau +++ /dev/null @@ -1,24 +0,0 @@ -local JestGlobals = require("@pkg/JestGlobals") -local test = JestGlobals.test -local expect = JestGlobals.expect - -local getEnv = require("./getEnv") - -test("returns a table", function() - expect(typeof(getEnv())).toBe("table") -end) - -test("has the correct 'script' global", function() - local env = getEnv(script.Parent.getEnv) - expect(env.script).toBe(script.Parent.getEnv) -end) - -test("sets _G to the 'globals' argument", function() - local globals = {} - local env = getEnv(script.Parent.getEnv, globals) - - expect(env._G).toBeDefined() - expect(env._G).toBe(globals) - -- selene: allow(global_usage) - expect(env._G).never.toBe(_G) -end) diff --git a/src/getInstanceFromFullName.luau b/src/getInstanceFromFullName.luau deleted file mode 100644 index 1486fe3..0000000 --- a/src/getInstanceFromFullName.luau +++ /dev/null @@ -1,89 +0,0 @@ ---[[ - Gets an instance based off the result of GetFullName(). - - This is used in conjunction with debug.info() to locate the calling script. - - Returns nil if the instance is outside the DataModel. -]] - -local Sift = require("@pkg/Sift") - -local PATH_SEPERATOR = "." - -local function canAccess(instance: Instance): boolean - local success = pcall(function() - return instance.Name - end) - - return success -end - -local function maybeGetService(serviceName: string): Instance? - local success, current: any = pcall(function() - return game:GetService(serviceName) - end) - - if success and current and canAccess(current) and current:IsA("Instance") then - return current - else - return nil - end -end - -local function getInstanceFromFullName(fullName: string): Instance? - local parts = fullName:split(PATH_SEPERATOR) - local serviceName = table.remove(parts, 1) - - if serviceName then - -- This function only works for instances in the DataModel. As such, the - -- first part of the path will always be a service, so if we can't find - -- one we exit out and return nil - local current = maybeGetService(serviceName) - - if current then - -- TODO: Verify this isn't also needed with the below TODO - -- if #parts == 1 then - -- return current - -- end - - while #parts > 0 do - -- Keep around a copy of the `parts` array. We are going to concat this - -- into new paths, and incrementally remove from the right to narrow - -- down the file path. - local tempParts = Sift.Array.copy(parts) - - -- The result of GetFullName() uses dots to separate paths, but we also - -- use dots in our file names (e.g. with spec and story files). As such, - -- this block will look ahead to see if multiple parts are actually a - -- single filename. - for _ = 1, #tempParts do - local name = table.concat(tempParts, PATH_SEPERATOR) - local found = current:FindFirstChild(name) - - if found then - current = found - parts = Sift.List.shift(parts, #name:split(PATH_SEPERATOR)) - break - else - if #tempParts == 1 then - -- TODO: Need this early exit to handle cases with instances not existing later on. Verify - -- if this can be effectively repro'd by calling this function on an instance that doesn't - -- exist. Particularly make sure to test look-aheads for `Foo.story`, it seemed like the - -- loop got stuck looking for "story" when "ChromeWindow.story" didn't exist - return nil - else - -- Reduce from the right until we find the next instance - tempParts = Sift.List.pop(tempParts) - end - end - end - end - - return current - end - end - - return nil -end - -return getInstanceFromFullName diff --git a/src/getInstanceFromFullName.spec.luau b/src/getInstanceFromFullName.spec.luau deleted file mode 100644 index 930fa8d..0000000 --- a/src/getInstanceFromFullName.spec.luau +++ /dev/null @@ -1,75 +0,0 @@ -local ReplicatedStorage = game:GetService("ReplicatedStorage") - -local JestGlobals = require("@pkg/JestGlobals") -local newFolder = require("@root/Testing/newFolder") - -local getInstanceFromFullName = require("./getInstanceFromFullName") - -local expect = JestGlobals.expect -local test = JestGlobals.test -local afterEach = JestGlobals.afterEach - -local folder: Folder - -afterEach(function() - if folder then - folder:Destroy() - end -end) - -test("gets services", function() - local path = getInstanceFromFullName("ReplicatedStorage") - expect(path).toBe(ReplicatedStorage) -end) - -test("works on nested instances", function() - local module = Instance.new("ModuleScript") - - folder = newFolder({ - foo = newFolder({ - bar = module, - }), - }) - folder.Parent = ReplicatedStorage - - local path = getInstanceFromFullName(module:GetFullName()) - expect(path).toBe(module) -end) - -test("works with spec files", function() - local module = Instance.new("ModuleScript") - - folder = newFolder({ - foo = newFolder({ - ["bar.spec"] = module, - }), - }) - folder.Parent = ReplicatedStorage - - local path = getInstanceFromFullName(module:GetFullName()) - expect(path).toBe(module) -end) - -test("finds spec files BEFORE the module it is associated with", function() - local module = Instance.new("ModuleScript") - - folder = newFolder({ - foo = newFolder({ - bar = Instance.new("ModuleScript"), - ["bar.spec"] = module, - }), - }) - folder.Parent = ReplicatedStorage - - local path = getInstanceFromFullName(module:GetFullName()) - expect(path).toBe(module) -end) - -test("returns nil if the first part of the path is not a service", function() - expect(getInstanceFromFullName("Part")).toBeUndefined() -end) - -test("returns nil when instance with an extension does not exist", function() - expect(getInstanceFromFullName("foo.story")).toBeUndefined() - expect(getInstanceFromFullName("Path.To.Foo.story")).toBeUndefined() -end) From 60c6c9480580dce5e017feb33a1f9ef0d1c26768 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Tue, 17 Dec 2024 16:57:01 -0800 Subject: [PATCH 10/28] Remove unused variable --- src/ModuleLoader.luau | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index d207c67..f56feb0 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -372,7 +372,6 @@ end ]=] function ModuleLoader:clear() self._moduleRegistry = {} - self._globals = {} for _, janitor in self._janitors do janitor:Cleanup() From 798cf1e4235e831a4a1f33ad64c24b98911f73e4 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 18 Dec 2024 09:16:20 -0800 Subject: [PATCH 11/28] Remove _loadModule, rename variable --- src/ModuleLoader.luau | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index f56feb0..c3df5ac 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -45,7 +45,6 @@ type ModuleLoaderImpl = { clearModule: (self: ModuleLoader, module: ModuleScript) -> (), clear: (self: ModuleLoader) -> (), - _loadModule: (self: ModuleLoader, loadedModule: LoadedModule) -> (), _execModule: (self: ModuleLoader, loadedModule: LoadedModule) -> (), _trackChanges: (self: ModuleLoader, module: ModuleScript) -> (), _getConsumers: (self: ModuleLoader, module: ModuleScript) -> { ModuleScript }, @@ -159,14 +158,14 @@ function ModuleLoader:cache(module: ModuleScript, result: any) self._moduleRegistry[module] = loadedModule end -function ModuleLoader:_execModule(localModule: LoadedModule) +function ModuleLoader:_execModule(loadedModule: LoadedModule) local moduleFunction, defaultEnvironment, errorMessage, cleanupFn - local module = localModule.instance + local module = loadedModule.instance - local loadedModule = self._loadedModuleFns[module] - if loadedModule then - moduleFunction = loadedModule[1] - defaultEnvironment = loadedModule[2] + local loadedModuleFns = self._loadedModuleFns[module] + if loadedModuleFns then + moduleFunction = loadedModuleFns[1] + defaultEnvironment = loadedModuleFns[2] else -- Narrowing this type here lets us appease the type checker while still -- counting on types for the rest of this file @@ -239,12 +238,7 @@ function ModuleLoader:_execModule(localModule: LoadedModule) self:_trackChanges(module) - localModule.exports = moduleResult[1] -end - -function ModuleLoader:_loadModule(localModule: LoadedModule) - self:_execModule(localModule) - localModule.isLoaded = true + loadedModule.exports = moduleResult[1] end --[=[ @@ -282,7 +276,8 @@ function ModuleLoader:require(module: ModuleScript): unknown self._moduleRegistry[module] = loadedModule local success, result = pcall(function() - self:_loadModule(loadedModule) + self:_execModule(loadedModule) + loadedModule.isLoaded = true end) if not success then self._moduleRegistry[module] = nil From 780568e1b5915718749e89654579a9beff2738f1 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Fri, 20 Dec 2024 15:17:47 -0800 Subject: [PATCH 12/28] Run tests with both loadmodule and loadstring --- src/ModuleLoader.luau | 12 +++++++----- src/ModuleLoader.spec.luau | 7 +++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index c3df5ac..a43cd92 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -28,7 +28,7 @@ type LoadedModule = { type ModuleLoaderProps = { _moduleRegistry: { [ModuleScript]: LoadedModule }, _loadedModuleFns: { [ModuleScript]: { any } }, - _loadstring: typeof(loadstring), + _forceLoadstring: boolean, _janitors: { [ModuleScript]: any }, _loadedModuleChangedBindable: BindableEvent, @@ -74,7 +74,7 @@ function ModuleLoader.new() self._moduleRegistry = {} self._loadedModuleFns = {} - self._loadstring = loadstring + self._forceLoadstring = false self._janitors = {} self._loadedModuleChangedBindable = Instance.new("BindableEvent") @@ -162,6 +162,8 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) local moduleFunction, defaultEnvironment, errorMessage, cleanupFn local module = loadedModule.instance + local shouldUseLoadmodule = self._forceLoadstring or not loadModuleEnabled + local loadedModuleFns = self._loadedModuleFns[module] if loadedModuleFns then moduleFunction = loadedModuleFns[1] @@ -169,10 +171,10 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) else -- Narrowing this type here lets us appease the type checker while still -- counting on types for the rest of this file - if loadModuleEnabled then + if shouldUseLoadmodule then moduleFunction, errorMessage, cleanupFn = loadmodule(module) else - moduleFunction = loadstring(getModuleSource(module), module:GetFullName()) + moduleFunction, errorMessage = loadstring(getModuleSource(module), module:GetFullName()) end assert(moduleFunction, errorMessage) @@ -197,7 +199,7 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) -- separate so that, in the future, `globalEnv` could expose these -- 'unmocked' functions instead of the ones in the global environment. local sandboxEnvironment = setmetatable({ - script = if loadModuleEnabled then defaultEnvironment.script else module, + script = if shouldUseLoadmodule then defaultEnvironment.script else module, game = defaultEnvironment.game, workspace = defaultEnvironment.workspace, plugin = defaultEnvironment.plugin, diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index d43f902..c2aa6cc 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -6,6 +6,7 @@ local expect = JestGlobals.expect local describe = JestGlobals.describe local beforeEach = JestGlobals.beforeEach local afterEach = JestGlobals.afterEach +local describeEach = describe.each :: any local ModuleLoader = require("./ModuleLoader") @@ -49,8 +50,13 @@ local mockModuleSource = {} local loader: ModuleLoader.ModuleLoader local tree +describeEach({ + "loadstring", + "loadmodule", +})("%s", function(loadingStrategy) beforeEach(function() loader = ModuleLoader.new() + loader._forceLoadstring = loadingStrategy == "loadstring" end) afterEach(function() @@ -659,5 +665,6 @@ describe("roblox-ts", function() expect(loader._moduleRegistry[mockRuntime:GetFullName()]).toBeUndefined() expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeUndefined() + end) end) end) From 3ad5ea0b3ce63d934ffc0438a74579f5ace72bd6 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Fri, 20 Dec 2024 16:09:52 -0800 Subject: [PATCH 13/28] Start working through test errors --- src/ModuleLoader.luau | 16 +- src/ModuleLoader.spec.luau | 697 ++++++++++++++++++------------------- wally.toml | 1 + 3 files changed, 346 insertions(+), 368 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index a43cd92..bcb052d 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -1,8 +1,11 @@ local Janitor = require("@pkg/Janitor") +local LuauPolyfill = require("@pkg/LuauPolyfill") local cleanLoadstringStack = require("./cleanLoadstringStack") local getRobloxTsRuntime = require("./getRobloxTsRuntime") +local Error = LuauPolyfill.Error + local loadmodule = (debug :: any).loadmodule local loadModuleEnabled = pcall(function() return loadmodule(Instance.new("ModuleScript")) @@ -175,9 +178,15 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) moduleFunction, errorMessage, cleanupFn = loadmodule(module) else moduleFunction, errorMessage = loadstring(getModuleSource(module), module:GetFullName()) + + if errorMessage then + errorMessage = cleanLoadstringStack(errorMessage) + end end - assert(moduleFunction, errorMessage) + if not moduleFunction then + error(Error.new(errorMessage)) + end -- Cache initial environment table to inherit from later defaultEnvironment = getfenv(moduleFunction) @@ -368,11 +377,12 @@ end ``` ]=] function ModuleLoader:clear() - self._moduleRegistry = {} - for _, janitor in self._janitors do janitor:Cleanup() end + + self._moduleRegistry = {} + self._loadedModuleFns = {} self._janitors = {} end diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index c2aa6cc..48e8170 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -10,7 +10,7 @@ local describeEach = describe.each :: any local ModuleLoader = require("./ModuleLoader") -local function countDict(dict: { [string]: any }) +local function countDict(dict: { [any]: any }) local count = 0 for _ in pairs(dict) do count += 1 @@ -54,552 +54,518 @@ describeEach({ "loadstring", "loadmodule", })("%s", function(loadingStrategy) -beforeEach(function() - loader = ModuleLoader.new() + beforeEach(function() + loader = ModuleLoader.new() loader._forceLoadstring = loadingStrategy == "loadstring" -end) + end) -afterEach(function() - loader:clear() + afterEach(function() + loader:clear() - if tree then - tree:Destroy() - end -end) + if tree then + tree:Destroy() + end + end) -describe("_trackChanges", function() - test("creates a Janitor instance if it doesn't exist", function() - local mockModuleInstance = Instance.new("ModuleScript") + describe("_trackChanges", function() + test("creates a Janitor instance if it doesn't exist", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - expect(loader._janitors[mockModuleInstance.Name]).toBeUndefined() + expect(loader._janitors[mockModuleInstance]).toBeUndefined() - loader:_trackChanges(mockModuleInstance) + loader:_trackChanges(mockModuleInstance) - expect(loader._janitors[mockModuleInstance.Name]).toBeDefined() - end) + expect(loader._janitors[mockModuleInstance]).toBeDefined() + end) - test("reuses the same Janitor instance for future calls", function() - local mockModuleInstance = Instance.new("ModuleScript") + test("reuses the same Janitor instance for future calls", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - loader:_trackChanges(mockModuleInstance) + loader:_trackChanges(mockModuleInstance) - local janitor = loader._janitors[mockModuleInstance.Name] + local janitor = loader._janitors[mockModuleInstance] - loader:_trackChanges(mockModuleInstance) + loader:_trackChanges(mockModuleInstance) - expect(loader._janitors[mockModuleInstance.Name]).toBe(janitor) + expect(loader._janitors[mockModuleInstance]).toBe(janitor) + end) end) -end) -describe("loadedModuleChanged", function() - test("fires when a required module has its ancestry changed", function() - local mockModuleInstance = Instance.new("ModuleScript") + describe("loadedModuleChanged", function() + test("fires when a required module has its ancestry changed", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - local wasFired = false + local wasFired = false - -- Parent the ModuleScript somewhere in the DataModel so we can - -- listen for AncestryChanged. - mockModuleInstance.Parent = game + -- Parent the ModuleScript somewhere in the DataModel so we can + -- listen for AncestryChanged. + mockModuleInstance.Parent = game - loader.loadedModuleChanged:Connect(function(other: ModuleScript) - if other == mockModuleInstance then - wasFired = true - end - end) + loader.loadedModuleChanged:Connect(function(other: ModuleScript) + if other == mockModuleInstance then + wasFired = true + end + end) - -- Require the module so that events get setup - loader:require(mockModuleInstance) + -- Require the module so that events get setup + loader:require(mockModuleInstance) - expect(wasFired).toBe(false) + expect(wasFired).toBe(false) - -- Trigger AncestryChanged to fire - mockModuleInstance.Parent = nil + -- Trigger AncestryChanged to fire + mockModuleInstance.Parent = nil - expect(wasFired).toBe(true) - end) + expect(wasFired).toBe(true) + end) - test("fires when a required module has its Source property change", function() - local mockModuleInstance = Instance.new("ModuleScript") + test("fires when a required module has its Source property change", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - local wasFired = false - loader.loadedModuleChanged:Connect(function(other: ModuleScript) - if other == mockModuleInstance then - wasFired = true - end - end) + local wasFired = false + loader.loadedModuleChanged:Connect(function(other: ModuleScript) + if other == mockModuleInstance then + wasFired = true + end + end) - -- Require the module so that events get setup - loader:require(mockModuleInstance) + -- Require the module so that events get setup + loader:require(mockModuleInstance) - expect(wasFired).toBe(false) + expect(wasFired).toBe(false) - mockModuleInstance.Source = "Something different" + mockModuleInstance.Source = "Something different" - expect(wasFired).toBe(true) - end) + expect(wasFired).toBe(true) + end) - test("fires for every consumer up the chain", function() - tree = createModuleTest({ - ModuleA = [[ + test("fires for every consumer up the chain", function() + tree = createModuleTest({ + ModuleA = [[ return "ModuleA" ]], - ModuleB = [[ + ModuleB = [[ require(script.Parent.ModuleA) return "ModuleB" ]], - ModuleC = [[ + ModuleC = [[ require(script.Parent.ModuleB) return "ModuleC" ]], - }) + }) - local count = 0 - loader.loadedModuleChanged:Connect(function(module) - for _, child in tree:GetChildren() do - if module == child then - count += 1 + local count = 0 + loader.loadedModuleChanged:Connect(function(module) + for _, child in tree:GetChildren() do + if module == child then + count += 1 + end end - end - end) + end) - loader:require(tree.ModuleC) + loader:require(tree.ModuleC) - tree.ModuleA.Source = "Changed" + tree.ModuleA.Source = "Changed" - expect(count).toBe(3) + expect(count).toBe(3) + end) end) -end) -describe("cache", function() - test("adds a module and its result to the cache", function() - local mockModuleInstance = Instance.new("ModuleScript") + describe("cache", function() + test("adds a module and its result to the cache", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - loader:cache(mockModuleInstance, mockModuleSource) + loader:cache(mockModuleInstance, mockModuleSource) - local cachedModule = loader._moduleRegistry[mockModuleInstance:GetFullName()] + local loadedModule = loader._moduleRegistry[mockModuleInstance] - expect(cachedModule).toBeDefined() - expect(cachedModule.result).toBe(mockModuleSource) + expect(loadedModule).toBeDefined() + expect(loadedModule.exports).toBe(mockModuleSource) + end) end) -end) -describe("require", function() - test("adds the module to the cache", function() - local mockModuleInstance = Instance.new("ModuleScript") + describe("require", function() + test("adds the module to the cache", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - loader:require(mockModuleInstance) - expect(loader._moduleRegistry[mockModuleInstance:GetFullName()]).toBeDefined() - end) + loader:require(mockModuleInstance) + expect(loader._moduleRegistry[mockModuleInstance]).toBeDefined() + end) - test("returns cached results", function() - tree = createModuleTest({ - -- We return a table since it can act as a unique symbol. So if - -- both consumers are getting the same table we can perform an - -- equality check - SharedModule = [[ + test("returns cached results", function() + tree = createModuleTest({ + -- We return a table since it can act as a unique symbol. So if + -- both consumers are getting the same table we can perform an + -- equality check + SharedModule = [[ local module = {} return module ]], - Consumer1 = [[ + Consumer1 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - Consumer2 = [[ + Consumer2 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - }) + }) - local sharedModuleFromConsumer1 = loader:require(tree.Consumer1) - local sharedModuleFromConsumer2 = loader:require(tree.Consumer2) + local sharedModuleFromConsumer1 = loader:require(tree.Consumer1) + local sharedModuleFromConsumer2 = loader:require(tree.Consumer2) - expect(sharedModuleFromConsumer1).toBe(sharedModuleFromConsumer2) - end) + expect(sharedModuleFromConsumer1).toBe(sharedModuleFromConsumer2) + end) - test("adds the calling script as a consumer", function() - tree = createModuleTest({ - SharedModule = [[ + test("adds the calling script as a consumer", function() + tree = createModuleTest({ + SharedModule = [[ local module = {} return module ]], - Consumer = [[ + Consumer = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - }) + }) - loader:require(tree.Consumer) + loader:require(tree.Consumer) - local cachedModule = loader._moduleRegistry[tree.SharedModule:GetFullName()] + local loadedModule = loader._moduleRegistry[tree.SharedModule] - expect(cachedModule).toBeDefined() - expect(cachedModule.consumers[tree.Consumer:GetFullName()]).toBeDefined() - end) + expect(loadedModule).toBeDefined() + expect(loadedModule.consumers[tree.Consumer]).toBeDefined() + end) - test("updates consumers when requiring a cached module from a different script", function() - tree = createModuleTest({ - SharedModule = [[ + test("updates consumers when requiring a cached module from a different script", function() + tree = createModuleTest({ + SharedModule = [[ local module = {} return module ]], - Consumer1 = [[ + Consumer1 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - Consumer2 = [[ + Consumer2 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - }) + }) - loader:require(tree.Consumer1) + loader:require(tree.Consumer1) - local cachedModule = loader._moduleRegistry[tree.SharedModule:GetFullName()] + local loadedModule = loader._moduleRegistry[tree.SharedModule] - expect(cachedModule.consumers[tree.Consumer1:GetFullName()]).toBeDefined() - expect(cachedModule.consumers[tree.Consumer2:GetFullName()]).toBeUndefined() + expect(loadedModule.consumers[tree.Consumer1]).toBeDefined() + expect(loadedModule.consumers[tree.Consumer2]).toBeUndefined() - loader:require(tree.Consumer2) + loader:require(tree.Consumer2) - expect(cachedModule.consumers[tree.Consumer1:GetFullName()]).toBeDefined() - expect(cachedModule.consumers[tree.Consumer2:GetFullName()]).toBeDefined() - end) + expect(loadedModule.consumers[tree.Consumer1]).toBeDefined() + expect(loadedModule.consumers[tree.Consumer2]).toBeDefined() + end) - test("keeps track of _G between modules", function() - tree = createModuleTest({ - WriteGlobal = [[ + test("keeps track of _G between modules", function() + tree = createModuleTest({ + WriteGlobal = [[ _G.foo = true return nil ]], - ReadGlobal = [[ + ReadGlobal = [[ return _G.foo ]], - }) + }) - loader:require(tree.WriteGlobal) + loader:require(tree.WriteGlobal) - expect(loader._globals.foo).toBe(true) + local result = loader:require(tree.ReadGlobal) - local result = loader:require(tree.ReadGlobal) - - expect(result).toBe(true) - end) + expect(result).toBe(true) + end) - test("keeps track of _G in nested requires", function() - tree = createModuleTest({ - DefineGlobal = [[ + test("keeps track of _G in nested requires", function() + tree = createModuleTest({ + DefineGlobal = [[ _G.foo = true return nil ]], - UseGlobal = [[ + UseGlobal = [[ require(script.Parent.DefineGlobal) return _G.foo ]], - }) - - local result = loader:require(tree.UseGlobal) + }) - expect(result).toBe(true) + local result = loader:require(tree.UseGlobal) - loader:clear() - - expect(loader._globals.foo).toBeUndefined() - end) + expect(result).toBe(true) + end) - test("adds globals on _G to the cachedModule's globals", function() - tree = createModuleTest({ - DefineGlobal = [[ - _G.foo = true - return nil + test("handles syntax errors for direct require", function() + tree = createModuleTest({ + Module = [[ + syntax error ]], - }) - - loader:require(tree.DefineGlobal) - - local cachedModule = loader._moduleRegistry[tree.DefineGlobal:GetFullName()] - expect(cachedModule.globals.foo).toBe(true) - end) + }) + tree.Name = "SyntaxError" - test("handles syntax errors for direct require", function() - tree = createModuleTest({ - Module = [[ - syntax error - ]], - }) - tree.Name = "SyntaxError" - - expect(function() - loader:require(tree.Module) - end).toThrow(`SyntaxError.Module:1: Incomplete statement: expected assignment or a function call`) - end) + expect(function() + loader:require(tree.Module) + end).toThrow(`SyntaxError.Module:1: Incomplete statement: expected assignment or a function call`) + end) - test("handles syntax errors for nested requires", function() - tree = createModuleTest({ - Module3 = [[ + test("handles syntax errors for nested requires", function() + tree = createModuleTest({ + Module3 = [[ syntax error ]], - Module2 = [[ + Module2 = [[ require(script.Parent.Module3) return {} ]], - Module1 = [[ + Module1 = [[ require(script.Parent.Module2) return {} ]], - Consumer = [[ + Consumer = [[ require(script.Parent.Module1) return nil ]], - }) - tree.Name = "SyntaxError" + }) + tree.Name = "SyntaxError" - expect(function() - loader:require(tree.Consumer) - end).toThrow(`SyntaxError.Consumer:1: Incomplete statement: expected assignment or a function call`) + expect(function() + loader:require(tree.Consumer) + end).toThrow(`SyntaxError.Module3:1: Incomplete statement: expected assignment or a function call`) + end) end) -end) -describe("clearModule", function() - test("clears a module from the cache", function() - tree = createModuleTest({ - Module = [[ + describe.skip("clearModule", function() + test("clears a module from the cache", function() + tree = createModuleTest({ + Module = [[ return "Module" ]], - }) + }) - loader:require(tree.Module) + loader:require(tree.Module) - expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Module]).toBeDefined() - loader:clearModule(tree.Module) + loader:clearModule(tree.Module) - expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeUndefined() - end) + expect(loader._moduleRegistry[tree.Module]).toBeUndefined() + end) - test("clears all consumers of a module from the cache", function() - tree = createModuleTest({ - SharedModule = [[ + test("clears all consumers of a module from the cache", function() + tree = createModuleTest({ + SharedModule = [[ local module = {} return module ]], - Consumer1 = [[ + Consumer1 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - Consumer2 = [[ + Consumer2 = [[ local sharedModule = require(script.Parent.SharedModule) return sharedModule ]], - }) + }) - loader:require(tree.Consumer1) - loader:require(tree.Consumer2) + loader:require(tree.Consumer1) + loader:require(tree.Consumer2) - expect(loader._moduleRegistry[tree.Consumer1:GetFullName()]).toBeDefined() - expect(loader._moduleRegistry[tree.Consumer2:GetFullName()]).toBeDefined() - expect(loader._moduleRegistry[tree.SharedModule:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.Consumer1]).toBeDefined() + expect(loader._moduleRegistry[tree.Consumer2]).toBeDefined() + expect(loader._moduleRegistry[tree.SharedModule]).toBeDefined() - loader:clearModule(tree.SharedModule) + loader:clearModule(tree.SharedModule) - expect(loader._moduleRegistry[tree.Consumer1:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.Consumer2:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.SharedModule:GetFullName()]).toBeUndefined() - end) + expect(loader._moduleRegistry[tree.Consumer1]).toBeUndefined() + expect(loader._moduleRegistry[tree.Consumer2]).toBeUndefined() + expect(loader._moduleRegistry[tree.SharedModule]).toBeUndefined() + end) - test("only clears modules in the consumer chain", function() - tree = createModuleTest({ - Module = [[ + test("only clears modules in the consumer chain", function() + tree = createModuleTest({ + Module = [[ return nil ]], - Consumer = [[ + Consumer = [[ require(script.Parent.Module) return nil ]], - Independent = [[ + Independent = [[ return nil ]], - }) - - loader:require(tree.Consumer) - loader:require(tree.Independent) - - expect(countDict(loader._moduleRegistry)).toBe(3) - - loader:clearModule(tree.Module) + }) - expect(countDict(loader._moduleRegistry)).toBe(1) - expect(loader._moduleRegistry[tree.Independent:GetFullName()]).toBeDefined() - end) - - test("clears all globals that a module supplied", function() - tree = createModuleTest({ - DefineGlobalFoo = [[ - _G.foo = true - return nil - ]], - DefineGlobalBar = [[ - _G.bar = false - return nil - ]], - }) + loader:require(tree.Consumer) + loader:require(tree.Independent) - loader:require(tree.DefineGlobalFoo) - loader:require(tree.DefineGlobalBar) + expect(countDict(loader._moduleRegistry)).toBe(3) - loader:clearModule(tree.DefineGlobalBar) + loader:clearModule(tree.Module) - expect(loader._globals.foo).toBeDefined() - expect(loader._globals.bar).toBeUndefined() - end) + expect(countDict(loader._moduleRegistry)).toBe(1) + expect(loader._moduleRegistry[tree.Independent]).toBeDefined() + end) - test("fires loadedModuleChanged when clearing a module", function() - tree = createModuleTest({ - Module = [[ + test("fires loadedModuleChanged when clearing a module", function() + tree = createModuleTest({ + Module = [[ return nil ]], - Consumer = [[ + Consumer = [[ require(script.Parent.Module) return nil ]], - }) + }) - local wasFired = false + local wasFired = false - loader.loadedModuleChanged:Connect(function() - wasFired = true - end) + loader.loadedModuleChanged:Connect(function() + wasFired = true + end) - loader:require(tree.Consumer) - loader:clearModule(tree.Consumer) + loader:require(tree.Consumer) + loader:clearModule(tree.Consumer) - expect(wasFired).toBe(true) - end) + expect(wasFired).toBe(true) + end) - test("fires loadedModuleChanged for every module up the chain", function() - tree = createModuleTest({ - Module3 = [[ + test("fires loadedModuleChanged for every module up the chain", function() + tree = createModuleTest({ + Module3 = [[ return {} ]], - Module2 = [[ + Module2 = [[ require(script.Parent.Module3) return {} ]], - Module1 = [[ + Module1 = [[ require(script.Parent.Module2) return {} ]], - Consumer = [[ + Consumer = [[ require(script.Parent.Module1) return nil ]], - }) + }) - local count = 0 + local count = 0 - loader.loadedModuleChanged:Connect(function() - count += 1 - end) + loader.loadedModuleChanged:Connect(function() + count += 1 + end) - loader:require(tree.Consumer) - loader:clearModule(tree.Module3) + loader:require(tree.Consumer) + loader:clearModule(tree.Module3) - expect(count).toBe(4) - end) + expect(count).toBe(4) + end) - test("never fires loadedModuleChanged for a module that hasn't been required", function() - local wasFired = false + test("never fires loadedModuleChanged for a module that hasn't been required", function() + local wasFired = false - loader.loadedModuleChanged:Connect(function() - wasFired = true - end) + loader.loadedModuleChanged:Connect(function() + wasFired = true + end) - -- Do nothing if the module hasn't been cached - local module = Instance.new("ModuleScript") - loader:clearModule(module) - expect(wasFired).toBe(false) + -- Do nothing if the module hasn't been cached + local module = Instance.new("ModuleScript") + loader:clearModule(module) + expect(wasFired).toBe(false) + end) end) -end) -describe("clear", function() - test("removes all modules from the cache", function() - local mockModuleInstance = Instance.new("ModuleScript") + describe.skip("clear", function() + test("removes all modules from the cache", function() + local mockModuleInstance = Instance.new("ModuleScript") + mockModuleInstance.Source = "return nil" - loader:cache(mockModuleInstance, mockModuleSource) + loader:cache(mockModuleInstance, mockModuleSource) - expect(countDict(loader._moduleRegistry)).toBe(1) + expect(countDict(loader._moduleRegistry)).toBe(1) - loader:clear() + loader:clear() - expect(countDict(loader._moduleRegistry)).toBe(0) - end) + expect(countDict(loader._moduleRegistry)).toBe(0) + end) - test("reset globals", function() - local globals = loader._globals + test("reset globals", function() + local globals = loader._globals - loader:clear() + loader:clear() - expect(loader._globals).never.toBe(globals) + expect(loader._globals).never.toBe(globals) + end) end) -end) -describe("consumers", function() - beforeEach(function() - tree = createModuleTest({ - ModuleA = [[ + describe.skip("consumers", function() + beforeEach(function() + tree = createModuleTest({ + ModuleA = [[ require(script.Parent.ModuleB) return "ModuleA" ]], - ModuleB = [[ + ModuleB = [[ return "ModuleB" ]], - ModuleC = [[ + ModuleC = [[ return "ModuleC" ]], - }) - end) + }) + end) - test("removes all consumers of a changed module from the cache", function() - loader:require(tree.ModuleA) + test("removes all consumers of a changed module from the cache", function() + loader:require(tree.ModuleA) - local hasItems = next(loader._moduleRegistry) ~= nil - expect(hasItems).toBe(true) + local hasItems = next(loader._moduleRegistry) ~= nil + expect(hasItems).toBe(true) - tree.ModuleB.Source = 'return "ModuleB Reloaded"' - task.wait() + tree.ModuleB.Source = 'return "ModuleB Reloaded"' + task.wait() - hasItems = next(loader._moduleRegistry) ~= nil - expect(hasItems).toBe(false) - end) + hasItems = next(loader._moduleRegistry) ~= nil + expect(hasItems).toBe(false) + end) - test("does not interfere with other cached modules", function() - loader:require(tree.ModuleA) - loader:require(tree.ModuleC) + test("does not interfere with other cached modules", function() + loader:require(tree.ModuleA) + loader:require(tree.ModuleC) - local hasItems = next(loader._moduleRegistry) ~= nil - expect(hasItems).toBe(true) + local hasItems = next(loader._moduleRegistry) ~= nil + expect(hasItems).toBe(true) - tree.ModuleB.Source = 'return "ModuleB Reloaded"' - task.wait() + tree.ModuleB.Source = 'return "ModuleB Reloaded"' + task.wait() - expect(loader._moduleRegistry[tree.ModuleA:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.ModuleB:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.ModuleC:GetFullName()]).toBeDefined() + expect(loader._moduleRegistry[tree.ModuleA]).toBeUndefined() + expect(loader._moduleRegistry[tree.ModuleB]).toBeUndefined() + expect(loader._moduleRegistry[tree.ModuleC]).toBeDefined() + end) end) -end) -describe("roblox-ts", function() - local rbxtsInclude - local mockRuntime + describe.skip("roblox-ts", function() + local rbxtsInclude + local mockRuntime - beforeEach(function() - rbxtsInclude = Instance.new("Folder") - rbxtsInclude.Name = "rbxts_include" + beforeEach(function() + rbxtsInclude = Instance.new("Folder") + rbxtsInclude.Name = "rbxts_include" - mockRuntime = Instance.new("ModuleScript") - mockRuntime.Name = "RuntimeLib" - mockRuntime.Source = [[ + mockRuntime = Instance.new("ModuleScript") + mockRuntime.Name = "RuntimeLib" + mockRuntime.Source = [[ local function import(...) return require(...) end @@ -607,64 +573,65 @@ describe("roblox-ts", function() import = import } ]] - mockRuntime.Parent = rbxtsInclude + mockRuntime.Parent = rbxtsInclude - rbxtsInclude.Parent = ReplicatedStorage - end) + rbxtsInclude.Parent = ReplicatedStorage + end) - afterEach(function() - loader:clear() - rbxtsInclude:Destroy() - end) + afterEach(function() + loader:clear() + rbxtsInclude:Destroy() + end) - test("clearModule() never clears the roblox-ts runtime from the cache", function() - -- This example isn't quite how a roblox-ts project would be setup since - -- the requires for `Shared` would be using `TS.import`, but it should - -- be close enough for our test case - tree = createModuleTest({ - Shared = [[ + test("clearModule() never clears the roblox-ts runtime from the cache", function() + -- This example isn't quite how a roblox-ts project would be setup since + -- the requires for `Shared` would be using `TS.import`, but it should + -- be close enough for our test case + tree = createModuleTest({ + Shared = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) return {} ]], - Module1 = [[ + Module1 = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) local Shared = TS.import(script.Parent.Shared) return nil ]], - Module2 = [[ + Module2 = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) local Shared = TS.import(script.Parent.Shared) return nil ]], - Root = [[ + Root = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) local Module1 = TS.import(script.Parent.Module1) local Module2 = TS.import(script.Parent.Module2) ]], - }) + }) - loader:require(tree.Root) - loader:clearModule(tree.Shared) + loader:require(tree.Root) + loader:clearModule(tree.Shared) - expect(loader._moduleRegistry[mockRuntime:GetFullName()]).toBeDefined() - expect(loader._moduleRegistry[tree.Shared:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module1:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module2:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.Root:GetFullName()]).toBeUndefined() - end) + expect(loader._moduleRegistry[mockRuntime]).toBeDefined() + expect(loader._moduleRegistry[tree.Shared]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module1]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module2]).toBeUndefined() + expect(loader._moduleRegistry[tree.Root]).toBeUndefined() + end) - test("clear() clears the roblox-ts runtime when calling", function() - tree = createModuleTest({ - Module = [[ + test("clear() clears the roblox-ts runtime when calling", function() + tree = createModuleTest({ + Module = [[ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) + return nil ]], - }) + }) - loader:require(tree.Module) - loader:clear() + loader:require(tree.Module) + loader:clear() - expect(loader._moduleRegistry[mockRuntime:GetFullName()]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module:GetFullName()]).toBeUndefined() + expect(loader._moduleRegistry[mockRuntime]).toBeUndefined() + expect(loader._moduleRegistry[tree.Module]).toBeUndefined() end) end) end) diff --git a/wally.toml b/wally.toml index df51e35..e1dd1e5 100644 --- a/wally.toml +++ b/wally.toml @@ -18,6 +18,7 @@ include = [ [dependencies] Janitor = "howmanysmall/janitor@1.13.15" Sift = "csqrl/sift@0.0.8" +LuauPolyfill = "jsdotlua/luau-polyfill@1.2.7" #[dev-dependencies] Jest = "jsdotlua/jest@3.6.1-rc.2" From 5492cddacecee7f1fc932bc15723eca4a5d12a35 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 22 Dec 2024 14:20:37 -0800 Subject: [PATCH 14/28] Track and run cleanup functions from loadmodule --- src/ModuleLoader.luau | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index bcb052d..005fa3b 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -31,6 +31,7 @@ type LoadedModule = { type ModuleLoaderProps = { _moduleRegistry: { [ModuleScript]: LoadedModule }, _loadedModuleFns: { [ModuleScript]: { any } }, + _cleanupFns: { () -> () }, _forceLoadstring: boolean, _janitors: { [ModuleScript]: any }, @@ -77,6 +78,7 @@ function ModuleLoader.new() self._moduleRegistry = {} self._loadedModuleFns = {} + self._cleanupFns = {} self._forceLoadstring = false self._janitors = {} self._loadedModuleChangedBindable = Instance.new("BindableEvent") @@ -191,7 +193,13 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) -- Cache initial environment table to inherit from later defaultEnvironment = getfenv(moduleFunction) + if self._loadedModuleFns then self._loadedModuleFns[module] = { moduleFunction, defaultEnvironment, cleanupFn } + else + if cleanupFn ~= nil then + table.insert(self._cleanupFns, cleanupFn) + end + end end -- The default behavior for function environments is to inherit the table @@ -381,8 +389,13 @@ function ModuleLoader:clear() janitor:Cleanup() end + for _, cleanupFn in self._cleanupFns do + cleanupFn() + end + self._moduleRegistry = {} self._loadedModuleFns = {} + self._cleanupFns = {} self._janitors = {} end From b4a1189b4fdc0ef9d69e84d017df65c85f7c6dbf Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 22 Dec 2024 14:24:21 -0800 Subject: [PATCH 15/28] Rename `module` variable to not step on the Lua global --- src/ModuleLoader.luau | 79 +++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 005fa3b..915f1d9 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -93,11 +93,11 @@ function ModuleLoader.new() ```lua local loader = ModuleLoader.new() - local result = loader:require(module) + local result = loader:require(moduleScript) loader.loadedModuleChanged:Connect(function() loader:clear() - result = loader:require(module) + result = loader:require(moduleScript) end) ``` @@ -118,23 +118,23 @@ end @private ]=] -function ModuleLoader:_trackChanges(module: ModuleScript) - local existingJanitor = self._janitors[module] +function ModuleLoader:_trackChanges(moduleScript: ModuleScript) + local existingJanitor = self._janitors[moduleScript] local janitor = if existingJanitor then existingJanitor else Janitor.new() janitor:Cleanup() - janitor:Add(module.AncestryChanged:Connect(function() - self:clearModule(module) + janitor:Add(moduleScript.AncestryChanged:Connect(function() + self:clearModule(moduleScript) end)) - janitor:Add(module.Changed:Connect(function(prop: string) + janitor:Add(moduleScript.Changed:Connect(function(prop: string) if prop == "Source" then - self:clearModule(module) + self:clearModule(moduleScript) end end)) - self._janitors[module] = janitor + self._janitors[moduleScript] = janitor end --[=[ @@ -146,40 +146,39 @@ end ```lua local moduleInstance = script.Parent.ModuleScript - local module = require(moduleInstance) + local moduleScript = require(moduleInstance) local loader = ModuleLoader.new() - loader:cache(moduleInstance, module) + loader:cache(moduleInstance, moduleScript) ``` ]=] -function ModuleLoader:cache(module: ModuleScript, result: any) +function ModuleLoader:cache(moduleScript: ModuleScript, result: any) local loadedModule: LoadedModule = { - instance = module, + instance = moduleScript, exports = result, isLoaded = true, - children = {}, + dependencies = {}, + consumers = {}, } - self._moduleRegistry[module] = loadedModule + self._moduleRegistry[moduleScript] = loadedModule end function ModuleLoader:_execModule(loadedModule: LoadedModule) local moduleFunction, defaultEnvironment, errorMessage, cleanupFn - local module = loadedModule.instance + local moduleScript = loadedModule.instance local shouldUseLoadmodule = self._forceLoadstring or not loadModuleEnabled - local loadedModuleFns = self._loadedModuleFns[module] + local loadedModuleFns = self._loadedModuleFns[moduleScript] if loadedModuleFns then moduleFunction = loadedModuleFns[1] defaultEnvironment = loadedModuleFns[2] else - -- Narrowing this type here lets us appease the type checker while still - -- counting on types for the rest of this file if shouldUseLoadmodule then - moduleFunction, errorMessage, cleanupFn = loadmodule(module) + moduleFunction, errorMessage, cleanupFn = loadmodule(moduleScript) else - moduleFunction, errorMessage = loadstring(getModuleSource(module), module:GetFullName()) + moduleFunction, errorMessage = loadstring(getModuleSource(moduleScript), moduleScript:GetFullName()) if errorMessage then errorMessage = cleanLoadstringStack(errorMessage) @@ -194,7 +193,7 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) defaultEnvironment = getfenv(moduleFunction) if self._loadedModuleFns then - self._loadedModuleFns[module] = { moduleFunction, defaultEnvironment, cleanupFn } + self._loadedModuleFns[moduleScript] = { moduleFunction, defaultEnvironment, cleanupFn } else if cleanupFn ~= nil then table.insert(self._cleanupFns, cleanupFn) @@ -216,7 +215,7 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) -- separate so that, in the future, `globalEnv` could expose these -- 'unmocked' functions instead of the ones in the global environment. local sandboxEnvironment = setmetatable({ - script = if shouldUseLoadmodule then defaultEnvironment.script else module, + script = if shouldUseLoadmodule then defaultEnvironment.script else moduleScript, game = defaultEnvironment.game, workspace = defaultEnvironment.workspace, plugin = defaultEnvironment.plugin, @@ -250,12 +249,12 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) error( string.format( "[Module Error]: %s did not return a valid result\n" .. "\tModuleScripts must return exactly one value", - tostring(module) + tostring(moduleScript) ) ) end - self:_trackChanges(module) + self:_trackChanges(moduleScript) loadedModule.exports = moduleResult[1] end @@ -272,12 +271,12 @@ end local module = loader:require(script.Parent.ModuleScript) ``` ]=] -function ModuleLoader:require(module: ModuleScript): unknown - if module.Name:find(".global$") then - return (require :: any)(module) +function ModuleLoader:require(moduleScript: ModuleScript): unknown + if moduleScript.Name:find(".global$") then + return (require :: any)(moduleScript) end - local existingModule = self._moduleRegistry[module] + local existingModule = self._moduleRegistry[moduleScript] if existingModule then return existingModule.exports end @@ -286,27 +285,27 @@ function ModuleLoader:require(module: ModuleScript): unknown -- circular dependencies that may arise while evaluating the module can -- be satisfied. local loadedModule: LoadedModule = { - instance = module, + instance = moduleScript, exports = nil, isLoaded = false, children = {}, } - self._moduleRegistry[module] = loadedModule + self._moduleRegistry[moduleScript] = loadedModule local success, result = pcall(function() self:_execModule(loadedModule) loadedModule.isLoaded = true end) if not success then - self._moduleRegistry[module] = nil + self._moduleRegistry[moduleScript] = nil error(result) end return loadedModule.exports end -function ModuleLoader:_getConsumers(module: ModuleScript): { ModuleScript } +function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript } local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) for consumer in loadedModule.children do local loadedChildModule = self._moduleRegistry[consumer] @@ -320,7 +319,7 @@ function ModuleLoader:_getConsumers(module: ModuleScript): { ModuleScript } end end - local loadedModule: LoadedModule = self._moduleRegistry[module] + local loadedModule: LoadedModule = self._moduleRegistry[moduleScript] local found = {} getConsumersRecursively(loadedModule, found) @@ -347,19 +346,19 @@ function ModuleLoader:clearModule(moduleToClear: ModuleScript) table.remove(modulesToClear, index) end - for _, module in modulesToClear do - local loadedModule = self._moduleRegistry[module] + for _, moduleScript in modulesToClear do + local loadedModule = self._moduleRegistry[moduleScript] if loadedModule then - self._moduleRegistry[module] = nil + self._moduleRegistry[moduleScript] = nil - local janitor = self._janitors[module] + local janitor = self._janitors[moduleScript] janitor:Cleanup() end end - for _, module in modulesToClear do - self._loadedModuleChangedBindable:Fire(module) + for _, moduleScript in modulesToClear do + self._loadedModuleChangedBindable:Fire(moduleScript) end end From ea587620110f53026aa77f02349f3c932ef8d847 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 22 Dec 2024 14:24:44 -0800 Subject: [PATCH 16/28] Track module dependencies --- src/ModuleLoader.luau | 13 ++++++++---- src/ModuleLoader.spec.luau | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 915f1d9..841ca42 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -6,7 +6,7 @@ local getRobloxTsRuntime = require("./getRobloxTsRuntime") local Error = LuauPolyfill.Error -local loadmodule = (debug :: any).loadmodule +local loadmodule: (ModuleScript) -> (() -> unknown?, string?, () -> ()) = debug["loadmodule"] local loadModuleEnabled = pcall(function() return loadmodule(Instance.new("ModuleScript")) end) @@ -25,7 +25,8 @@ type LoadedModule = { instance: ModuleScript, isLoaded: boolean, exports: LoadedModuleExports, - children: { [ModuleScript]: boolean }, + dependencies: { [ModuleScript]: boolean }, + consumers: { [ModuleScript]: boolean }, } type ModuleLoaderProps = { @@ -230,6 +231,9 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) -- deeper support in Jest. error("Require-by-string is not enabled for use inside Jest at this time.") end + + loadedModule.dependencies[otherModule] = true + return self:require(otherModule) end, }, { @@ -288,7 +292,8 @@ function ModuleLoader:require(moduleScript: ModuleScript): unknown instance = moduleScript, exports = nil, isLoaded = false, - children = {}, + dependencies = {}, + consumers = {}, } self._moduleRegistry[moduleScript] = loadedModule @@ -307,7 +312,7 @@ end function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript } local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) - for consumer in loadedModule.children do + for consumer in loadedModule.consumers do local loadedChildModule = self._moduleRegistry[consumer] if loadedChildModule then diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index 48e8170..5d95f56 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -197,6 +197,47 @@ describeEach({ expect(loader._moduleRegistry[mockModuleInstance]).toBeDefined() end) + test("keeps track of module dependencies", function() + tree = createModuleTest({ + Module3 = [[ + return nil + ]], + Module2 = [[ + require(script.Parent.Module3) + return nil + ]], + Module1 = [[ + require(script.Parent.Module2) + return nil + ]], + Root = [[ + require(script.Parent.Module1) + return nil + ]], + }) + + loader:require(tree.Root) + + expect(loader._moduleRegistry[tree.Root]).toMatchObject({ + dependencies = { + [tree.Module1] = true, + }, + }) + expect(loader._moduleRegistry[tree.Module1]).toMatchObject({ + dependencies = { + [tree.Module2] = true, + }, + }) + expect(loader._moduleRegistry[tree.Module2]).toMatchObject({ + dependencies = { + [tree.Module3] = true, + }, + }) + expect(loader._moduleRegistry[tree.Module3]).toMatchObject({ + dependencies = {}, + }) + end) + test("returns cached results", function() tree = createModuleTest({ -- We return a table since it can act as a unique symbol. So if From 5df4cc2391b600e7fb11041d5a7ba148c68570aa Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 22 Dec 2024 16:23:18 -0800 Subject: [PATCH 17/28] Handle module consumers --- src/ModuleLoader.luau | 79 ++++++++++++++++++------------ src/ModuleLoader.spec.luau | 63 ++++++++++++------------ src/createModuleRegistry.luau | 44 +++++++++++++++++ src/createModuleRegistry.spec.luau | 62 +++++++++++++++++++++++ src/getCallerPath.luau | 32 ++++++++++++ src/types.luau | 19 +++++++ 6 files changed, 237 insertions(+), 62 deletions(-) create mode 100644 src/createModuleRegistry.luau create mode 100644 src/createModuleRegistry.spec.luau create mode 100644 src/getCallerPath.luau create mode 100644 src/types.luau diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 841ca42..c32a0ee 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -2,7 +2,13 @@ local Janitor = require("@pkg/Janitor") local LuauPolyfill = require("@pkg/LuauPolyfill") local cleanLoadstringStack = require("./cleanLoadstringStack") +local createModuleRegistry = require("./createModuleRegistry") +local getCallerPath = require("./getCallerPath") local getRobloxTsRuntime = require("./getRobloxTsRuntime") +local types = require("./types") + +type ModuleRegistry = types.ModuleRegistry +type LoadedModule = types.LoadedModule local Error = LuauPolyfill.Error @@ -19,18 +25,8 @@ local function getModuleSource(moduleScript: ModuleScript): string return if success then result else "" end -type LoadedModuleExports = unknown - -type LoadedModule = { - instance: ModuleScript, - isLoaded: boolean, - exports: LoadedModuleExports, - dependencies: { [ModuleScript]: boolean }, - consumers: { [ModuleScript]: boolean }, -} - type ModuleLoaderProps = { - _moduleRegistry: { [ModuleScript]: LoadedModule }, + _moduleRegistry: ModuleRegistry, _loadedModuleFns: { [ModuleScript]: { any } }, _cleanupFns: { () -> () }, _forceLoadstring: boolean, @@ -77,7 +73,7 @@ ModuleLoader.__index = ModuleLoader function ModuleLoader.new() local self = {} - self._moduleRegistry = {} + self._moduleRegistry = createModuleRegistry() self._loadedModuleFns = {} self._cleanupFns = {} self._forceLoadstring = false @@ -162,7 +158,7 @@ function ModuleLoader:cache(moduleScript: ModuleScript, result: any) consumers = {}, } - self._moduleRegistry[moduleScript] = loadedModule + self._moduleRegistry.add(moduleScript, loadedModule) end function ModuleLoader:_execModule(loadedModule: LoadedModule) @@ -280,8 +276,21 @@ function ModuleLoader:require(moduleScript: ModuleScript): unknown return (require :: any)(moduleScript) end - local existingModule = self._moduleRegistry[moduleScript] + local caller: ModuleScript? + local callerPath = getCallerPath() + if callerPath then + local loadedCallerModule = self._moduleRegistry.getByFullName(callerPath) + if loadedCallerModule and loadedCallerModule.instance then + caller = loadedCallerModule.instance + end + end + + local existingModule = self._moduleRegistry.getByInstance(moduleScript) if existingModule then + if caller then + existingModule.consumers[caller] = true + end + return existingModule.exports end @@ -293,17 +302,21 @@ function ModuleLoader:require(moduleScript: ModuleScript): unknown exports = nil, isLoaded = false, dependencies = {}, - consumers = {}, + consumers = if caller + then { + [caller] = true, + } + else {}, } - self._moduleRegistry[moduleScript] = loadedModule + self._moduleRegistry.add(moduleScript, loadedModule) local success, result = pcall(function() self:_execModule(loadedModule) loadedModule.isLoaded = true end) if not success then - self._moduleRegistry[moduleScript] = nil + self._moduleRegistry.remove(moduleScript) error(result) end @@ -313,7 +326,7 @@ end function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript } local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) for consumer in loadedModule.consumers do - local loadedChildModule = self._moduleRegistry[consumer] + local loadedChildModule = self._moduleRegistry.getByInstance(consumer) if loadedChildModule then if not found[loadedChildModule.instance] then @@ -324,22 +337,26 @@ function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript end end - local loadedModule: LoadedModule = self._moduleRegistry[moduleScript] - local found = {} + local loadedModule: LoadedModule? = self._moduleRegistry.getByInstance(moduleScript) - getConsumersRecursively(loadedModule, found) + if loadedModule then + local found = {} - local consumers = {} - for consumer in found do - table.insert(consumers, consumer) - end + getConsumersRecursively(loadedModule, found) - return consumers + local consumers = {} + for consumer in found do + table.insert(consumers, consumer) + end + + return consumers + else + return {} + end end --- function ModuleLoader:clearModule(moduleToClear: ModuleScript) - if not self._moduleRegistry[moduleToClear] then + if not self._moduleRegistry.getByInstance(moduleToClear) then return end @@ -352,10 +369,10 @@ function ModuleLoader:clearModule(moduleToClear: ModuleScript) end for _, moduleScript in modulesToClear do - local loadedModule = self._moduleRegistry[moduleScript] + local loadedModule = self._moduleRegistry.getByInstance(moduleScript) if loadedModule then - self._moduleRegistry[moduleScript] = nil + self._moduleRegistry.remove(moduleScript) local janitor = self._janitors[moduleScript] janitor:Cleanup() @@ -397,7 +414,7 @@ function ModuleLoader:clear() cleanupFn() end - self._moduleRegistry = {} + self._moduleRegistry.reset() self._loadedModuleFns = {} self._cleanupFns = {} self._janitors = {} diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index 5d95f56..579efb9 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -181,9 +181,9 @@ describeEach({ loader:cache(mockModuleInstance, mockModuleSource) - local loadedModule = loader._moduleRegistry[mockModuleInstance] + local loadedModule = loader._moduleRegistry.getByInstance(mockModuleInstance) - expect(loadedModule).toBeDefined() + assert(loadedModule, "loaded module undefined") expect(loadedModule.exports).toBe(mockModuleSource) end) end) @@ -194,7 +194,7 @@ describeEach({ mockModuleInstance.Source = "return nil" loader:require(mockModuleInstance) - expect(loader._moduleRegistry[mockModuleInstance]).toBeDefined() + expect(loader._moduleRegistry.getByInstance(mockModuleInstance)).toBeDefined() end) test("keeps track of module dependencies", function() @@ -218,22 +218,22 @@ describeEach({ loader:require(tree.Root) - expect(loader._moduleRegistry[tree.Root]).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Root)).toMatchObject({ dependencies = { [tree.Module1] = true, }, }) - expect(loader._moduleRegistry[tree.Module1]).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module1)).toMatchObject({ dependencies = { [tree.Module2] = true, }, }) - expect(loader._moduleRegistry[tree.Module2]).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module2)).toMatchObject({ dependencies = { [tree.Module3] = true, }, }) - expect(loader._moduleRegistry[tree.Module3]).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module3)).toMatchObject({ dependencies = {}, }) end) @@ -277,9 +277,8 @@ describeEach({ loader:require(tree.Consumer) - local loadedModule = loader._moduleRegistry[tree.SharedModule] - - expect(loadedModule).toBeDefined() + local loadedModule = loader._moduleRegistry.getByInstance(tree.SharedModule) + assert(loadedModule, "loaded module undefined") expect(loadedModule.consumers[tree.Consumer]).toBeDefined() end) @@ -301,7 +300,9 @@ describeEach({ loader:require(tree.Consumer1) - local loadedModule = loader._moduleRegistry[tree.SharedModule] + local loadedModule = loader._moduleRegistry.getByInstance(tree.SharedModule) + + assert(loadedModule, "loaded module undefined") expect(loadedModule.consumers[tree.Consumer1]).toBeDefined() expect(loadedModule.consumers[tree.Consumer2]).toBeUndefined() @@ -396,11 +397,11 @@ describeEach({ loader:require(tree.Module) - expect(loader._moduleRegistry[tree.Module]).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeDefined() loader:clearModule(tree.Module) - expect(loader._moduleRegistry[tree.Module]).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeUndefined() end) test("clears all consumers of a module from the cache", function() @@ -422,15 +423,15 @@ describeEach({ loader:require(tree.Consumer1) loader:require(tree.Consumer2) - expect(loader._moduleRegistry[tree.Consumer1]).toBeDefined() - expect(loader._moduleRegistry[tree.Consumer2]).toBeDefined() - expect(loader._moduleRegistry[tree.SharedModule]).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.Consumer1)).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.Consumer2)).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.SharedModule)).toBeDefined() loader:clearModule(tree.SharedModule) - expect(loader._moduleRegistry[tree.Consumer1]).toBeUndefined() - expect(loader._moduleRegistry[tree.Consumer2]).toBeUndefined() - expect(loader._moduleRegistry[tree.SharedModule]).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Consumer1)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Consumer2)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.SharedModule)).toBeUndefined() end) test("only clears modules in the consumer chain", function() @@ -455,7 +456,7 @@ describeEach({ loader:clearModule(tree.Module) expect(countDict(loader._moduleRegistry)).toBe(1) - expect(loader._moduleRegistry[tree.Independent]).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.Independent)).toBeDefined() end) test("fires loadedModuleChanged when clearing a module", function() @@ -549,7 +550,7 @@ describeEach({ end) end) - describe.skip("consumers", function() + describe("consumers", function() beforeEach(function() tree = createModuleTest({ ModuleA = [[ @@ -590,9 +591,9 @@ describeEach({ tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - expect(loader._moduleRegistry[tree.ModuleA]).toBeUndefined() - expect(loader._moduleRegistry[tree.ModuleB]).toBeUndefined() - expect(loader._moduleRegistry[tree.ModuleC]).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.ModuleA)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.ModuleB)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.ModuleC)).toBeDefined() end) end) @@ -653,11 +654,11 @@ describeEach({ loader:require(tree.Root) loader:clearModule(tree.Shared) - expect(loader._moduleRegistry[mockRuntime]).toBeDefined() - expect(loader._moduleRegistry[tree.Shared]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module1]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module2]).toBeUndefined() - expect(loader._moduleRegistry[tree.Root]).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(mockRuntime)).toBeDefined() + expect(loader._moduleRegistry.getByInstance(tree.Shared)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module1)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module2)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Root)).toBeUndefined() end) test("clear() clears the roblox-ts runtime when calling", function() @@ -671,8 +672,8 @@ describeEach({ loader:require(tree.Module) loader:clear() - expect(loader._moduleRegistry[mockRuntime]).toBeUndefined() - expect(loader._moduleRegistry[tree.Module]).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(mockRuntime)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeUndefined() end) end) end) diff --git a/src/createModuleRegistry.luau b/src/createModuleRegistry.luau new file mode 100644 index 0000000..e059cc7 --- /dev/null +++ b/src/createModuleRegistry.luau @@ -0,0 +1,44 @@ +local types = require("./types") + +type LoadedModule = types.LoadedModule +type ModuleRegistry = types.ModuleRegistry + +local function createModuleRegistry(): ModuleRegistry + local registry = { + byInstance = {}, + byPath = {}, + } + + local function add(moduleScript: ModuleScript, loadedModule: LoadedModule) + registry.byInstance[moduleScript] = loadedModule + registry.byPath[moduleScript:GetFullName()] = loadedModule + end + + local function remove(moduleScript: ModuleScript) + registry.byInstance[moduleScript] = nil + registry.byPath[moduleScript:GetFullName()] = nil + end + + local function reset() + table.clear(registry.byInstance) + table.clear(registry.byPath) + end + + local function getByInstance(moduleScript: ModuleScript): LoadedModule? + return registry.byInstance[moduleScript] + end + + local function getByFullName(fullName: string): LoadedModule? + return registry.byPath[fullName] + end + + return { + add = add, + remove = remove, + reset = reset, + getByInstance = getByInstance, + getByFullName = getByFullName, + } +end + +return createModuleRegistry diff --git a/src/createModuleRegistry.spec.luau b/src/createModuleRegistry.spec.luau new file mode 100644 index 0000000..4dc4d9e --- /dev/null +++ b/src/createModuleRegistry.spec.luau @@ -0,0 +1,62 @@ +local JestGlobals = require("@pkg/JestGlobals") +local test = JestGlobals.test +local expect = JestGlobals.expect + +local createModuleRegistry = require("./createModuleRegistry") + +test("adds a module to the registry", function() + local registry = createModuleRegistry() + + local moduleScript = Instance.new("ModuleScript") + local mockLoadedModule = {} :: any + + registry.add(moduleScript, mockLoadedModule) + + expect(registry.getByInstance(moduleScript)).toBeDefined() + expect(registry.getByFullName(moduleScript:GetFullName())).toBeDefined() +end) + +test("removes a module from the registry", function() + local registry = createModuleRegistry() + + local moduleScript = Instance.new("ModuleScript") + local mockLoadedModule = {} :: any + + registry.add(moduleScript, mockLoadedModule) + + expect(registry.getByInstance(moduleScript)).toBeDefined() + expect(registry.getByFullName(moduleScript:GetFullName())).toBeDefined() + + registry.remove(moduleScript) + + expect(registry.getByInstance(moduleScript)).toBeUndefined() + expect(registry.getByFullName(moduleScript:GetFullName())).toBeUndefined() +end) + +test("reset the registry", function() + local registry = createModuleRegistry() + + local folder = Instance.new("Folder") + + for _, name in { "ModuleA", "ModuleB", "ModuleC" } do + local moduleScript = Instance.new("ModuleScript") + moduleScript.Name = name + moduleScript.Parent = folder + + local mockLoadedModule = {} :: any + + registry.add(moduleScript, mockLoadedModule) + end + + for _, moduleScript in folder:GetChildren() do + expect(registry.getByInstance(moduleScript :: ModuleScript)).toBeDefined() + expect(registry.getByFullName(moduleScript:GetFullName())).toBeDefined() + end + + registry.reset() + + for _, moduleScript in folder:GetChildren() do + expect(registry.getByInstance(moduleScript :: ModuleScript)).toBeUndefined() + expect(registry.getByFullName(moduleScript:GetFullName())).toBeUndefined() + end +end) diff --git a/src/getCallerPath.luau b/src/getCallerPath.luau new file mode 100644 index 0000000..921c3f5 --- /dev/null +++ b/src/getCallerPath.luau @@ -0,0 +1,32 @@ +local root = script.Parent + +local LOADSTRING_PATH_PATTERN = '%[string "(.*)"%]' + +local function getCallerPath(): string? + local level = 1 + + while true do + local path = debug.info(level, "s") + + if path then + -- Skip over any path that is a descendant of this package + if not path:find(root.Name, nil, true) then + -- Sometimes the path is represented as `[string "path.to.module"]` + -- so we match for the instance path and, if found, return it + local pathFromLoadstring = path:match(LOADSTRING_PATH_PATTERN) + + if pathFromLoadstring then + return pathFromLoadstring + else + return path + end + end + else + return nil + end + + level += 1 + end +end + +return getCallerPath diff --git a/src/types.luau b/src/types.luau new file mode 100644 index 0000000..a157e1c --- /dev/null +++ b/src/types.luau @@ -0,0 +1,19 @@ +export type LoadedModuleExports = unknown + +export type LoadedModule = { + instance: ModuleScript, + isLoaded: boolean, + exports: LoadedModuleExports, + dependencies: { [ModuleScript]: boolean }, + consumers: { [ModuleScript]: boolean }, +} + +export type ModuleRegistry = { + add: (moduleScript: ModuleScript, loadedModule: LoadedModule) -> (), + remove: (moduleScript: ModuleScript) -> (), + reset: () -> (), + getByInstance: (moduleScript: ModuleScript) -> LoadedModule?, + getByFullName: (fullName: string) -> LoadedModule?, +} + +return nil From c16954b3e91c37d456523f767b00104092921325 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 22 Dec 2024 16:31:12 -0800 Subject: [PATCH 18/28] Fix up some more tests --- src/ModuleLoader.spec.luau | 38 +++++++++++------------------------ src/createModuleRegistry.luau | 9 +++++++++ src/types.luau | 1 + 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index 579efb9..a4d4c67 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -10,14 +10,6 @@ local describeEach = describe.each :: any local ModuleLoader = require("./ModuleLoader") -local function countDict(dict: { [any]: any }) - local count = 0 - for _ in pairs(dict) do - count += 1 - end - return count -end - type ModuleTestTree = { [string]: string | ModuleTestTree, } @@ -387,7 +379,7 @@ describeEach({ end) end) - describe.skip("clearModule", function() + describe("clearModule", function() test("clears a module from the cache", function() tree = createModuleTest({ Module = [[ @@ -451,11 +443,11 @@ describeEach({ loader:require(tree.Consumer) loader:require(tree.Independent) - expect(countDict(loader._moduleRegistry)).toBe(3) + expect(#loader._moduleRegistry.getAllModules()).toBe(3) loader:clearModule(tree.Module) - expect(countDict(loader._moduleRegistry)).toBe(1) + expect(#loader._moduleRegistry.getAllModules()).toBe(1) expect(loader._moduleRegistry.getByInstance(tree.Independent)).toBeDefined() end) @@ -527,26 +519,18 @@ describeEach({ end) end) - describe.skip("clear", function() + describe("clear", function() test("removes all modules from the cache", function() local mockModuleInstance = Instance.new("ModuleScript") mockModuleInstance.Source = "return nil" loader:cache(mockModuleInstance, mockModuleSource) - expect(countDict(loader._moduleRegistry)).toBe(1) - - loader:clear() - - expect(countDict(loader._moduleRegistry)).toBe(0) - end) - - test("reset globals", function() - local globals = loader._globals + expect(#loader._moduleRegistry.getAllModules()).toBe(1) loader:clear() - expect(loader._globals).never.toBe(globals) + expect(#loader._moduleRegistry.getAllModules()).toBe(0) end) end) @@ -571,13 +555,13 @@ describeEach({ test("removes all consumers of a changed module from the cache", function() loader:require(tree.ModuleA) - local hasItems = next(loader._moduleRegistry) ~= nil + local hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - hasItems = next(loader._moduleRegistry) ~= nil + hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(false) end) @@ -585,7 +569,7 @@ describeEach({ loader:require(tree.ModuleA) loader:require(tree.ModuleC) - local hasItems = next(loader._moduleRegistry) ~= nil + local hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' @@ -597,7 +581,7 @@ describeEach({ end) end) - describe.skip("roblox-ts", function() + describe("roblox-ts", function() local rbxtsInclude local mockRuntime @@ -648,6 +632,8 @@ describeEach({ local TS = require(game:GetService("ReplicatedStorage").rbxts_include.RuntimeLib) local Module1 = TS.import(script.Parent.Module1) local Module2 = TS.import(script.Parent.Module2) + + return nil ]], }) diff --git a/src/createModuleRegistry.luau b/src/createModuleRegistry.luau index e059cc7..7afd9d5 100644 --- a/src/createModuleRegistry.luau +++ b/src/createModuleRegistry.luau @@ -19,6 +19,14 @@ local function createModuleRegistry(): ModuleRegistry registry.byPath[moduleScript:GetFullName()] = nil end + local function getAllModules(): { ModuleScript } + local modules = {} + for moduleScript in registry.byInstance do + table.insert(modules, moduleScript) + end + return modules + end + local function reset() table.clear(registry.byInstance) table.clear(registry.byPath) @@ -36,6 +44,7 @@ local function createModuleRegistry(): ModuleRegistry add = add, remove = remove, reset = reset, + getAllModules = getAllModules, getByInstance = getByInstance, getByFullName = getByFullName, } diff --git a/src/types.luau b/src/types.luau index a157e1c..f9a2526 100644 --- a/src/types.luau +++ b/src/types.luau @@ -12,6 +12,7 @@ export type ModuleRegistry = { add: (moduleScript: ModuleScript, loadedModule: LoadedModule) -> (), remove: (moduleScript: ModuleScript) -> (), reset: () -> (), + getAllModules: () -> { ModuleScript }, getByInstance: (moduleScript: ModuleScript) -> LoadedModule?, getByFullName: (fullName: string) -> LoadedModule?, } From 3c1d3075ee2ccc972b9ae77e3c758c5c73cd44b8 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 23 Dec 2024 16:05:22 -0800 Subject: [PATCH 19/28] Don't run with loadmodule in CI --- src/ModuleLoader.luau | 2 +- src/ModuleLoader.spec.luau | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index c32a0ee..efeaf0c 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -165,7 +165,7 @@ function ModuleLoader:_execModule(loadedModule: LoadedModule) local moduleFunction, defaultEnvironment, errorMessage, cleanupFn local moduleScript = loadedModule.instance - local shouldUseLoadmodule = self._forceLoadstring or not loadModuleEnabled + local shouldUseLoadmodule = not self._forceLoadstring and loadModuleEnabled local loadedModuleFns = self._loadedModuleFns[moduleScript] if loadedModuleFns then diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index a4d4c67..c594968 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -10,6 +10,10 @@ local describeEach = describe.each :: any local ModuleLoader = require("./ModuleLoader") +local loadModuleEnabled = pcall(function() + return debug["loadmodule"](Instance.new("ModuleScript")) +end) + type ModuleTestTree = { [string]: string | ModuleTestTree, } @@ -46,6 +50,10 @@ describeEach({ "loadstring", "loadmodule", })("%s", function(loadingStrategy) + if loadingStrategy == "loadmodule" and not loadModuleEnabled then + test = test.skip + end + beforeEach(function() loader = ModuleLoader.new() loader._forceLoadstring = loadingStrategy == "loadstring" From 78511642bb10ac35356b46e10fb1ee7a4254196f Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 23 Dec 2024 16:16:33 -0800 Subject: [PATCH 20/28] Fix consumer recursion --- src/ModuleLoader.luau | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index efeaf0c..85b1e77 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -331,7 +331,7 @@ function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript if loadedChildModule then if not found[loadedChildModule.instance] then found[loadedChildModule.instance] = true - getConsumersRecursively(loadedModule, found) + getConsumersRecursively(loadedChildModule, found) end end end From 2899708c7777eec9c628b20adb319f6508e6557b Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 23 Dec 2024 16:20:30 -0800 Subject: [PATCH 21/28] Fix analysis errors --- src/ModuleLoader.spec.luau | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ModuleLoader.spec.luau b/src/ModuleLoader.spec.luau index c594968..06ab183 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/ModuleLoader.spec.luau @@ -51,7 +51,7 @@ describeEach({ "loadmodule", })("%s", function(loadingStrategy) if loadingStrategy == "loadmodule" and not loadModuleEnabled then - test = test.skip + test = test.skip :: any end beforeEach(function() @@ -223,17 +223,17 @@ describeEach({ [tree.Module1] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module1)).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toMatchObject({ dependencies = { [tree.Module2] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module2)).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toMatchObject({ dependencies = { [tree.Module3] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module3)).toMatchObject({ + expect(loader._moduleRegistry.getByInstance(tree.Module3 :: ModuleScript)).toMatchObject({ dependencies = {}, }) end) @@ -508,7 +508,7 @@ describeEach({ end) loader:require(tree.Consumer) - loader:clearModule(tree.Module3) + loader:clearModule(tree.Module3 :: ModuleScript) expect(count).toBe(4) end) @@ -650,8 +650,8 @@ describeEach({ expect(loader._moduleRegistry.getByInstance(mockRuntime)).toBeDefined() expect(loader._moduleRegistry.getByInstance(tree.Shared)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Module1)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Module2)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toBeUndefined() + expect(loader._moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toBeUndefined() expect(loader._moduleRegistry.getByInstance(tree.Root)).toBeUndefined() end) From ac453fb79333f3ffb2fc8ecd86a322b27b56e493 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Mon, 23 Dec 2024 16:22:03 -0800 Subject: [PATCH 22/28] Remove unused test helper --- src/Testing/newFolder.luau | 13 ------------ src/Testing/newFolder.spec.luau | 36 --------------------------------- 2 files changed, 49 deletions(-) delete mode 100644 src/Testing/newFolder.luau delete mode 100644 src/Testing/newFolder.spec.luau diff --git a/src/Testing/newFolder.luau b/src/Testing/newFolder.luau deleted file mode 100644 index 2675b69..0000000 --- a/src/Testing/newFolder.luau +++ /dev/null @@ -1,13 +0,0 @@ -local function newFolder(children: { [string]: Instance }): Folder - local folder = Instance.new("Folder") - folder.Name = "Root" - - for name, child in pairs(children) do - child.Name = name - child.Parent = folder - end - - return folder -end - -return newFolder diff --git a/src/Testing/newFolder.spec.luau b/src/Testing/newFolder.spec.luau deleted file mode 100644 index 32ce1ba..0000000 --- a/src/Testing/newFolder.spec.luau +++ /dev/null @@ -1,36 +0,0 @@ -local JestGlobals = require("@pkg/JestGlobals") -local newFolder = require("./newFolder") - -local expect = JestGlobals.expect -local test = JestGlobals.test - -test("return a folder named 'Root'", function() - local folder = newFolder({}) - expect(folder:IsA("Folder")).toBe(true) - expect(folder.Name).toBe("Root") -end) - -test("name children after the dictionary keys", function() - local child1 = Instance.new("Part") - local child2 = Instance.new("Model") - - local folder: any = newFolder({ - Child1 = child1, - Child2 = child2, - }) - - expect(folder.Child1).toBe(child1) - expect(folder.Child2).toBe(child2) -end) - -test("support nesting newFolder as children", function() - local folder: any = newFolder({ - Child = newFolder({ - AnotherChild = newFolder({ - Module = Instance.new("ModuleScript"), - }), - }), - }) - - expect(folder.Child.AnotherChild.Module).toBeDefined() -end) From a0c51856f0f22e5ec851d7a5b34c8331ac4f084e Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 25 Dec 2024 20:30:20 -0800 Subject: [PATCH 23/28] Guve getModuleSource its own file --- src/ModuleLoader.luau | 9 +-------- src/getModuleSource.luau | 9 +++++++++ 2 files changed, 10 insertions(+), 8 deletions(-) create mode 100644 src/getModuleSource.luau diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 6d2ec38..beed205 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -4,6 +4,7 @@ local LuauPolyfill = require("@pkg/LuauPolyfill") local cleanLoadstringStack = require("./cleanLoadstringStack") local createModuleRegistry = require("./createModuleRegistry") local getCallerPath = require("./getCallerPath") +local getModuleSource = require("./getModuleSource") local getRobloxTsRuntime = require("./getRobloxTsRuntime") local types = require("./types") @@ -17,14 +18,6 @@ local loadModuleEnabled = pcall(function() return loadmodule(Instance.new("ModuleScript")) end) -local function getModuleSource(moduleScript: ModuleScript): string - local success, result = pcall(function() - return moduleScript.Source - end) - - return if success then result else "" -end - type ModuleLoaderProps = { _moduleRegistry: ModuleRegistry, _loadedModuleFns: { [ModuleScript]: { any } }, diff --git a/src/getModuleSource.luau b/src/getModuleSource.luau new file mode 100644 index 0000000..3466078 --- /dev/null +++ b/src/getModuleSource.luau @@ -0,0 +1,9 @@ +local function getModuleSource(moduleScript: ModuleScript): string + local success, result = pcall(function() + return moduleScript.Source + end) + + return if success then result else "" +end + +return getModuleSource From 002f47f440400a83364e0c5b9802bc25eec9b312 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 25 Dec 2024 20:32:23 -0800 Subject: [PATCH 24/28] Minor reverts --- src/ModuleLoader.luau | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index beed205..e15094c 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -83,11 +83,11 @@ function ModuleLoader.new() ```lua local loader = ModuleLoader.new() - local result = loader:require(moduleScript) + local result = loader:require(module) loader.loadedModuleChanged:Connect(function() loader:clear() - result = loader:require(moduleScript) + result = loader:require(module) end) ``` @@ -264,7 +264,7 @@ end local module = loader:require(script.Parent.ModuleScript) ``` ]=] -function ModuleLoader:require(moduleScript: ModuleScript): unknown +function ModuleLoader:require(moduleScript: ModuleScript) if moduleScript.Name:find(".global$") then return (require :: any)(moduleScript) end From c65004da8dc96d9d42d6b6040556c32c63cc9db0 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 25 Dec 2024 20:32:31 -0800 Subject: [PATCH 25/28] Move some types around --- src/ModuleLoader.luau | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index e15094c..f6946e7 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -8,9 +8,6 @@ local getModuleSource = require("./getModuleSource") local getRobloxTsRuntime = require("./getRobloxTsRuntime") local types = require("./types") -type ModuleRegistry = types.ModuleRegistry -type LoadedModule = types.LoadedModule - local Error = LuauPolyfill.Error local loadmodule: (ModuleScript) -> (() -> unknown?, string?, () -> ()) = debug["loadmodule"] @@ -18,6 +15,9 @@ local loadModuleEnabled = pcall(function() return loadmodule(Instance.new("ModuleScript")) end) +type ModuleRegistry = types.ModuleRegistry +type LoadedModule = types.LoadedModule + type ModuleLoaderProps = { _moduleRegistry: ModuleRegistry, _loadedModuleFns: { [ModuleScript]: { any } }, From 6f52468b184f565399276598e68c71f990911ca4 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Wed, 25 Dec 2024 20:52:36 -0800 Subject: [PATCH 26/28] Add upstream link --- src/ModuleLoader.luau | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index f6946e7..276c932 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -155,6 +155,9 @@ function ModuleLoader:cache(moduleScript: ModuleScript, result: any) end function ModuleLoader:_execModule(loadedModule: LoadedModule) + -- This method is adapted from: + -- https://github.com/Roblox/jest-roblox/blob/408eac/src/jest-runtime/src/init.lua#L1847-L2102 + local moduleFunction, defaultEnvironment, errorMessage, cleanupFn local moduleScript = loadedModule.instance From ffeb4851eea0c9c46fe2e9420713d03451780de8 Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sat, 28 Dec 2024 21:28:09 -0800 Subject: [PATCH 27/28] Make sure to delete the old build dir before testing --- .lune/test-cloud.luau | 1 + 1 file changed, 1 insertion(+) diff --git a/.lune/test-cloud.luau b/.lune/test-cloud.luau index 755ea6f..7be749a 100644 --- a/.lune/test-cloud.luau +++ b/.lune/test-cloud.luau @@ -10,6 +10,7 @@ local apiKey = assert(args.apiKey, "--apiKey must be supplied with a valid Open local testPlaceFile = "test-place.rbxl" +run("rm", { "-rf", project.BUILD_PATH }) compile("dev") run("rojo", { "build", project.ROJO_TESTS_PROJECT, "-o", testPlaceFile }) From e338481a18a7c25bfc1a61230815385e5e4a880e Mon Sep 17 00:00:00 2001 From: Marin Minnerly Date: Sun, 29 Dec 2024 10:55:58 -0800 Subject: [PATCH 28/28] Take a functional approach to ModuleLoader --- src/ModuleLoader.luau | 313 +------------ src/createModuleLoader.luau | 412 ++++++++++++++++++ ...spec.luau => createModuleLoader.spec.luau} | 207 +++++---- src/init.luau | 4 +- src/types.luau | 20 +- 5 files changed, 547 insertions(+), 409 deletions(-) create mode 100644 src/createModuleLoader.luau rename src/{ModuleLoader.spec.luau => createModuleLoader.spec.luau} (70%) diff --git a/src/ModuleLoader.luau b/src/ModuleLoader.luau index 276c932..23d4bf9 100644 --- a/src/ModuleLoader.luau +++ b/src/ModuleLoader.luau @@ -1,50 +1,25 @@ -local Janitor = require("@pkg/Janitor") -local LuauPolyfill = require("@pkg/LuauPolyfill") - -local cleanLoadstringStack = require("./cleanLoadstringStack") -local createModuleRegistry = require("./createModuleRegistry") -local getCallerPath = require("./getCallerPath") -local getModuleSource = require("./getModuleSource") -local getRobloxTsRuntime = require("./getRobloxTsRuntime") +local createModuleLoader = require("./createModuleLoader") local types = require("./types") -local Error = LuauPolyfill.Error - -local loadmodule: (ModuleScript) -> (() -> unknown?, string?, () -> ()) = debug["loadmodule"] -local loadModuleEnabled = pcall(function() - return loadmodule(Instance.new("ModuleScript")) -end) - -type ModuleRegistry = types.ModuleRegistry -type LoadedModule = types.LoadedModule +type ModuleLoader = types.ModuleLoader type ModuleLoaderProps = { - _moduleRegistry: ModuleRegistry, - _loadedModuleFns: { [ModuleScript]: { any } }, - _cleanupFns: { () -> () }, - _forceLoadstring: boolean, - _janitors: { [ModuleScript]: any }, - - _loadedModuleChangedBindable: BindableEvent, + _loader: ModuleLoader, loadedModuleChanged: RBXScriptSignal, } type ModuleLoaderImpl = { __index: ModuleLoaderImpl, - new: () -> ModuleLoader, - - require: (self: ModuleLoader, moduleScript: ModuleScript) -> any, - cache: (self: ModuleLoader, moduleScript: ModuleScript, result: any) -> (), - clearModule: (self: ModuleLoader, moduleScript: ModuleScript) -> (), - clear: (self: ModuleLoader) -> (), + new: () -> ModuleLoaderClass, - _execModule: (self: ModuleLoader, loadedModule: LoadedModule) -> (), - _trackChanges: (self: ModuleLoader, moduleScript: ModuleScript) -> (), - _getConsumers: (self: ModuleLoader, moduleScript: ModuleScript) -> { ModuleScript }, + require: (self: ModuleLoaderClass, moduleScript: ModuleScript) -> any, + cache: (self: ModuleLoaderClass, moduleScript: ModuleScript, result: any) -> (), + clearModule: (self: ModuleLoaderClass, moduleScript: ModuleScript) -> (), + clear: (self: ModuleLoaderClass) -> (), } -export type ModuleLoader = typeof(setmetatable({} :: ModuleLoaderProps, {} :: ModuleLoaderImpl)) +export type ModuleLoaderClass = typeof(setmetatable({} :: ModuleLoaderProps, {} :: ModuleLoaderImpl)) --[=[ ModuleScript loader that bypasses Roblox's require cache. @@ -66,12 +41,7 @@ ModuleLoader.__index = ModuleLoader function ModuleLoader.new() local self = {} - self._moduleRegistry = createModuleRegistry() - self._loadedModuleFns = {} - self._cleanupFns = {} - self._forceLoadstring = false - self._janitors = {} - self._loadedModuleChangedBindable = Instance.new("BindableEvent") + self._loader = createModuleLoader() --[=[ Fired when any ModuleScript required through this class has its ancestry @@ -94,39 +64,11 @@ function ModuleLoader.new() @prop loadedModuleChanged RBXScriptSignal @within ModuleLoader ]=] - self.loadedModuleChanged = self._loadedModuleChangedBindable.Event + self.loadedModuleChanged = self._loader.loadedModuleChanged return setmetatable(self, ModuleLoader) end ---[=[ - Tracks the changes to a required module's ancestry and `Source`. - - When ancestry or `Source` changes, the `loadedModuleChanged` event is fired. - When this happens, the user should clear the cache and require the root - module again to reload. - - @private -]=] -function ModuleLoader:_trackChanges(moduleScript: ModuleScript) - local existingJanitor = self._janitors[moduleScript] - local janitor = if existingJanitor then existingJanitor else Janitor.new() - - janitor:Cleanup() - - janitor:Add(moduleScript.AncestryChanged:Connect(function() - self:clearModule(moduleScript) - end)) - - janitor:Add(moduleScript.Changed:Connect(function(prop: string) - if prop == "Source" then - self:clearModule(moduleScript) - end - end)) - - self._janitors[moduleScript] = janitor -end - --[=[ Set the cached value for a module before it is loaded. @@ -143,116 +85,7 @@ end ``` ]=] function ModuleLoader:cache(moduleScript: ModuleScript, result: any) - local loadedModule: LoadedModule = { - instance = moduleScript, - exports = result, - isLoaded = true, - dependencies = {}, - consumers = {}, - } - - self._moduleRegistry.add(moduleScript, loadedModule) -end - -function ModuleLoader:_execModule(loadedModule: LoadedModule) - -- This method is adapted from: - -- https://github.com/Roblox/jest-roblox/blob/408eac/src/jest-runtime/src/init.lua#L1847-L2102 - - local moduleFunction, defaultEnvironment, errorMessage, cleanupFn - local moduleScript = loadedModule.instance - - local shouldUseLoadmodule = not self._forceLoadstring and loadModuleEnabled - - local loadedModuleFns = self._loadedModuleFns[moduleScript] - if loadedModuleFns then - moduleFunction = loadedModuleFns[1] - defaultEnvironment = loadedModuleFns[2] - else - if shouldUseLoadmodule then - moduleFunction, errorMessage, cleanupFn = loadmodule(moduleScript) - else - moduleFunction, errorMessage = loadstring(getModuleSource(moduleScript), moduleScript:GetFullName()) - - if errorMessage then - errorMessage = cleanLoadstringStack(errorMessage) - end - end - - if not moduleFunction then - error(Error.new(errorMessage)) - end - - -- Cache initial environment table to inherit from later - defaultEnvironment = getfenv(moduleFunction) - - if self._loadedModuleFns then - self._loadedModuleFns[moduleScript] = { moduleFunction, defaultEnvironment, cleanupFn } - else - if cleanupFn ~= nil then - table.insert(self._cleanupFns, cleanupFn) - end - end - end - - -- The default behavior for function environments is to inherit the table - -- instance from the parent environment. This means that each invocation of - -- `moduleFunction()` will return a new module instance but with the same - -- environment table as `moduleFunction` itself at the time of invocation. - -- In order to properly sanbox module instances, we need to ensure that each - -- instance has its own distinct environment table containing the specific - -- overrides for it, but still inherits from the default parent environment - -- for non-overriden environment goodies. - - -- This is the 'least mocked' environment that scripts will be able to see. - -- The final function environment inherits from this sandbox. This is - -- separate so that, in the future, `globalEnv` could expose these - -- 'unmocked' functions instead of the ones in the global environment. - local sandboxEnvironment = setmetatable({ - script = if shouldUseLoadmodule then defaultEnvironment.script else moduleScript, - game = defaultEnvironment.game, - workspace = defaultEnvironment.workspace, - plugin = defaultEnvironment.plugin, - - -- legacy aliases for data model - Game = defaultEnvironment.game, - Workspace = defaultEnvironment.workspace, - - require = function(otherModule: ModuleScript | string) - if typeof(otherModule) == "string" then - -- Disabling this at the surface level of the API until we have - -- deeper support in Jest. - error("Require-by-string is not enabled for use inside Jest at this time.") - end - - loadedModule.dependencies[otherModule] = true - - return self:require(otherModule) - end, - }, { - __index = defaultEnvironment, - }) - - -- This is the environment actually passed to scripts, including all global - -- mocks and other customisations the user might choose to apply. - local mockedSandboxEnvironment = setmetatable({}, { - __index = sandboxEnvironment, - }) - - setfenv(moduleFunction, mockedSandboxEnvironment :: any) - local moduleResult = table.pack(moduleFunction()) - - if moduleResult.n ~= 1 then - error( - string.format( - "[Module Error]: %s did not return a valid result\n" .. "\tModuleScripts must return exactly one value", - tostring(moduleScript) - ) - ) - end - - self:_trackChanges(moduleScript) - - loadedModule.exports = moduleResult[1] + self._loader.cache(moduleScript, result) end --[=[ @@ -268,116 +101,11 @@ end ``` ]=] function ModuleLoader:require(moduleScript: ModuleScript) - if moduleScript.Name:find(".global$") then - return (require :: any)(moduleScript) - end - - local caller: ModuleScript? - local callerPath = getCallerPath() - if callerPath then - local loadedCallerModule = self._moduleRegistry.getByFullName(callerPath) - if loadedCallerModule and loadedCallerModule.instance then - caller = loadedCallerModule.instance - end - end - - local existingModule = self._moduleRegistry.getByInstance(moduleScript) - if existingModule then - if caller then - existingModule.consumers[caller] = true - end - - return existingModule.exports - end - - -- We must register the pre-allocated module object first so that any - -- circular dependencies that may arise while evaluating the module can - -- be satisfied. - local loadedModule: LoadedModule = { - instance = moduleScript, - exports = nil, - isLoaded = false, - dependencies = {}, - consumers = if caller - then { - [caller] = true, - } - else {}, - } - - self._moduleRegistry.add(moduleScript, loadedModule) - - local success, result = pcall(function() - self:_execModule(loadedModule) - loadedModule.isLoaded = true - end) - if not success then - self._moduleRegistry.remove(moduleScript) - error(result) - end - - return loadedModule.exports -end - -function ModuleLoader:_getConsumers(moduleScript: ModuleScript): { ModuleScript } - local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) - for consumer in loadedModule.consumers do - local loadedChildModule = self._moduleRegistry.getByInstance(consumer) - - if loadedChildModule then - if not found[loadedChildModule.instance] then - found[loadedChildModule.instance] = true - getConsumersRecursively(loadedChildModule, found) - end - end - end - end - - local loadedModule: LoadedModule? = self._moduleRegistry.getByInstance(moduleScript) - - if loadedModule then - local found = {} - - getConsumersRecursively(loadedModule, found) - - local consumers = {} - for consumer in found do - table.insert(consumers, consumer) - end - - return consumers - else - return {} - end + return self._loader.require(moduleScript) end function ModuleLoader:clearModule(moduleToClear: ModuleScript) - if not self._moduleRegistry.getByInstance(moduleToClear) then - return - end - - local consumers = self:_getConsumers(moduleToClear) - local modulesToClear = { moduleToClear, table.unpack(consumers) } - - local index = table.find(modulesToClear, getRobloxTsRuntime()) - if index then - table.remove(modulesToClear, index) - end - - for _, moduleScript in modulesToClear do - local loadedModule = self._moduleRegistry.getByInstance(moduleScript) - - if loadedModule then - self._moduleRegistry.remove(moduleScript) - - local janitor = self._janitors[moduleScript] - janitor:Cleanup() - end - end - - for _, moduleScript in modulesToClear do - self._loadedModuleChangedBindable:Fire(moduleScript) - end + self._loader.clearModule(moduleToClear) end --[=[ @@ -402,18 +130,7 @@ end ``` ]=] function ModuleLoader:clear() - for _, janitor in self._janitors do - janitor:Cleanup() - end - - for _, cleanupFn in self._cleanupFns do - cleanupFn() - end - - self._moduleRegistry.reset() - self._loadedModuleFns = {} - self._cleanupFns = {} - self._janitors = {} + self._loader.clear() end return ModuleLoader diff --git a/src/createModuleLoader.luau b/src/createModuleLoader.luau new file mode 100644 index 0000000..096f32b --- /dev/null +++ b/src/createModuleLoader.luau @@ -0,0 +1,412 @@ +local Janitor = require("@pkg/Janitor") +local LuauPolyfill = require("@pkg/LuauPolyfill") + +local cleanLoadstringStack = require("./cleanLoadstringStack") +local createModuleRegistry = require("./createModuleRegistry") +local getCallerPath = require("./getCallerPath") +local getModuleSource = require("./getModuleSource") +local getRobloxTsRuntime = require("./getRobloxTsRuntime") +local types = require("./types") + +local Error = LuauPolyfill.Error + +type LoadedModule = types.LoadedModule +type LoadedModuleExports = types.LoadedModuleExports +type LoadingStrategy = types.LoadingStrategy +type LoadModuleFn = types.LoadModuleFn +type ModuleLoader = types.ModuleLoader +type ModuleRegistry = types.ModuleRegistry + +local loadmodule: (ModuleScript) -> (() -> LoadedModuleExports, string?, () -> ()) = debug["loadmodule"] +local loadModuleEnabled = pcall(function() + return loadmodule(Instance.new("ModuleScript")) +end) + +--[=[ + ModuleScript loader that bypasses Roblox's require cache. + + This class aims to solve a common problem where code needs to be run in + Studio, but once a change is made to an already required module the whole + place must be reloaded for the cache to be reset. With this class, the cache + is ignored when requiring a module so you are able to load a module, make + changes, and load it again without reloading the whole place. + + @class ModuleLoader +]=] +function createModuleLoader(): ModuleLoader + local moduleRegistry = createModuleRegistry() + local loadedModuleFns: { [ModuleScript]: { any } } = {} + local cleanupFns: { () -> () } = {} + local loadingStrategy: LoadingStrategy = "Automatic" + local janitors: { [ModuleScript]: any } = {} + + local function _getModuleRegistry() + return moduleRegistry + end + + local function setLoadingStrategy(strategy: LoadingStrategy) + loadingStrategy = strategy + end + + --[=[ + Fired when any ModuleScript required through this class has its ancestry + or `Source` property changed. This applies to the ModuleScript passed to + `ModuleLoader:require()` and every module that it subsequently requirs. + + This event is useful for reloading a module when it or any of it + dependencies change. + + ```lua + local loader = createModuleLoader() + local result = loader.require(module) + + loader.loadedModuleChanged:Connect(function() + loader.clear() + result = loader.require(module) + end) + ``` + + @prop loadedModuleChanged RBXScriptSignal + @within ModuleLoader + ]=] + local loadedModuleChanged = Instance.new("BindableEvent") + + local loadModule: LoadModuleFn + + local function getConsumers(moduleScript: ModuleScript): { ModuleScript } + local function getConsumersRecursively(loadedModule: LoadedModule, found: { [ModuleScript]: true }) + for consumer in loadedModule.consumers do + local loadedChildModule = moduleRegistry.getByInstance(consumer) + + if loadedChildModule then + if not found[loadedChildModule.instance] then + found[loadedChildModule.instance] = true + getConsumersRecursively(loadedChildModule, found) + end + end + end + end + + local loadedModule: LoadedModule? = moduleRegistry.getByInstance(moduleScript) + + if loadedModule then + local found = {} + + getConsumersRecursively(loadedModule, found) + + local consumers = {} + for consumer in found do + table.insert(consumers, consumer) + end + + return consumers + else + return {} + end + end + + local function clearModule(moduleToClear: ModuleScript) + if not moduleRegistry.getByInstance(moduleToClear) then + return + end + + local consumers = getConsumers(moduleToClear) + local modulesToClear = { moduleToClear, table.unpack(consumers) } + + local index = table.find(modulesToClear, getRobloxTsRuntime()) + if index then + table.remove(modulesToClear, index) + end + + for _, moduleScript in modulesToClear do + local loadedModule = moduleRegistry.getByInstance(moduleScript) + + if loadedModule then + local janitor = janitors[moduleScript] + janitor:Cleanup() + end + end + + for _, moduleScript in modulesToClear do + loadedModuleChanged:Fire(moduleScript) + end + end + + --[=[ + Tracks the changes to a required module's ancestry and `Source`. + + When ancestry or `Source` changes, the `loadedModuleChanged` event is fired. + When this happens, the user should clear the cache and require the root + module again to reload. + + @private + ]=] + local function trackChanges(moduleScript: ModuleScript) + local existingJanitor = janitors[moduleScript] + local janitor = if existingJanitor then existingJanitor else Janitor.new() + + janitor:Cleanup() + + janitor:Add(moduleScript.AncestryChanged:Connect(function() + clearModule(moduleScript) + end)) + + janitor:Add(moduleScript.Changed:Connect(function(prop: string) + if prop == "Source" then + clearModule(moduleScript) + end + end)) + + janitor:Add(function() + moduleRegistry.remove(moduleScript) + loadedModuleFns[moduleScript] = nil + end) + + janitors[moduleScript] = janitor + end + + --[=[ + Set the cached value for a module before it is loaded. + + This is useful is very specific situations. For example, this method is + used to cache a copy of Roact so that when a module is loaded with this + class it uses the same table instance. + + ```lua + local moduleInstance = script.Parent.ModuleScript + local moduleScript = require(moduleInstance) + + local loader = createModuleLoader() + loader.cache(moduleInstance, moduleScript) + ``` + ]=] + local function cache(moduleScript: ModuleScript, result: any) + local loadedModule: LoadedModule = { + instance = moduleScript, + exports = result, + isLoaded = true, + dependencies = {}, + consumers = {}, + } + + moduleRegistry.add(moduleScript, loadedModule) + end + + local function execModule(loadedModule: LoadedModule) + -- This method is adapted from: + -- https://github.com/Roblox/jest-roblox/blob/408eac/src/jest-runtime/src/init.lua#L1847-L2102 + + local moduleFunction, defaultEnvironment, errorMessage, cleanupFn + local moduleScript = loadedModule.instance + + local shouldUseLoadmodule = loadingStrategy == "LoadModule" + or (loadingStrategy == "Automatic" and loadModuleEnabled) + + local existingLoadedModuleFns = loadedModuleFns[moduleScript] + if existingLoadedModuleFns then + moduleFunction = existingLoadedModuleFns[1] + defaultEnvironment = existingLoadedModuleFns[2] + else + if shouldUseLoadmodule then + moduleFunction, errorMessage, cleanupFn = loadmodule(moduleScript) + else + moduleFunction, errorMessage = loadstring(getModuleSource(moduleScript), moduleScript:GetFullName()) + + if errorMessage then + errorMessage = cleanLoadstringStack(errorMessage) + end + end + + if not moduleFunction then + error(Error.new(errorMessage)) + end + + -- Cache initial environment table to inherit from later + defaultEnvironment = getfenv(moduleFunction) + + if loadedModuleFns then + loadedModuleFns[moduleScript] = { moduleFunction, defaultEnvironment, cleanupFn } + else + if cleanupFn ~= nil then + table.insert(cleanupFns, cleanupFn) + end + end + end + + -- The default behavior for function environments is to inherit the table + -- instance from the parent environment. This means that each invocation of + -- `moduleFunction()` will return a new module instance but with the same + -- environment table as `moduleFunction` loadModule the time of invocation. + -- In order to properly sanbox module instances, we need to ensure that each + -- instance has its own distinct environment table containing the specific + -- overrides for it, but still inherits from the default parent environment + -- for non-overriden environment goodies. + + -- This is the 'least mocked' environment that scripts will be able to see. + -- The final function environment inherits from this sandbox. This is + -- separate so that, in the future, `globalEnv` could expose these + -- 'unmocked' functions instead of the ones in the global environment. + local sandboxEnvironment = setmetatable({ + script = if shouldUseLoadmodule then defaultEnvironment.script else moduleScript, + game = defaultEnvironment.game, + workspace = defaultEnvironment.workspace, + plugin = defaultEnvironment.plugin, + + -- legacy aliases for data model + Game = defaultEnvironment.game, + Workspace = defaultEnvironment.workspace, + + require = function(otherModule: ModuleScript | string) + if typeof(otherModule) == "string" then + -- Disabling this at the surface level of the API until we have + -- deeper support in Jest. + error("Require-by-string is not enabled for use inside Jest at this time.") + end + + loadedModule.dependencies[otherModule] = true + + return loadModule(otherModule) + end, + }, { + __index = defaultEnvironment, + }) + + -- This is the environment actually passed to scripts, including all global + -- mocks and other customisations the user might choose to apply. + local mockedSandboxEnvironment = setmetatable({}, { + __index = sandboxEnvironment, + }) + + setfenv(moduleFunction, mockedSandboxEnvironment :: any) + local moduleResult = table.pack(moduleFunction()) + + if moduleResult.n ~= 1 then + error( + string.format( + "[Module Error]: %s did not return a valid result\n" + .. "\tModuleScripts must return exactly one value", + tostring(moduleScript) + ) + ) + end + + trackChanges(moduleScript) + + loadedModule.exports = moduleResult[1] + end + + --[=[ + Require a module with a fresh ModuleScript require cache. + + This method is functionally the same as running `require(script.Parent.ModuleScript)`, + however in this case the module is not cached. As such, if a change occurs + to the module you can call this method again to get the latest changes. + + ```lua + local loader = createModuleLoader() + local module = loader.require(script.Parent.ModuleScript) + ``` + ]=] + function loadModule(moduleScript: ModuleScript) + if moduleScript.Name:find(".global$") then + return (require :: any)(moduleScript) + end + + local caller: ModuleScript? + local callerPath = getCallerPath() + if callerPath then + local loadedCallerModule = moduleRegistry.getByFullName(callerPath) + if loadedCallerModule and loadedCallerModule.instance then + caller = loadedCallerModule.instance + end + end + + local existingModule = moduleRegistry.getByInstance(moduleScript) + if existingModule then + if caller then + existingModule.consumers[caller] = true + end + + return existingModule.exports + end + + -- We must register the pre-allocated module object first so that any + -- circular dependencies that may arise while evaluating the module can + -- be satisfied. + local loadedModule: LoadedModule = { + instance = moduleScript, + exports = nil, + isLoaded = false, + dependencies = {}, + consumers = if caller + then { + [caller] = true, + } + else {}, + } + + moduleRegistry.add(moduleScript, loadedModule) + + local success, result = pcall(function() + execModule(loadedModule) + loadedModule.isLoaded = true + end) + if not success then + moduleRegistry.remove(moduleScript) + error(result) + end + + return loadedModule.exports + end + + --[=[ + Clears out the internal cache. + + While this module bypasses Roblox's ModuleScript cache, one is still + maintained internally so that repeated requires to the same module return a + cached value. + + This method should be called when you need to require a module again. i.e. + if the module's Source has been changed. + + ```lua + local loader = createModuleLoader() + loader.require(script.Parent.ModuleScript) + + -- Later... + + -- Clear the cache and require the module again + loader.clear() + loader.require(script.Parent.ModuleScript) + ``` + ]=] + local function clear() + for _, janitor in janitors do + janitor:Cleanup() + end + + for _, cleanupFn in cleanupFns do + cleanupFn() + end + + moduleRegistry.reset() + loadedModuleFns = {} + cleanupFns = {} + janitors = {} + end + + return { + _getModuleRegistry = _getModuleRegistry, + + cache = cache, + loadModule = loadModule, + require = loadModule, + clearModule = clearModule, + clear = clear, + setLoadingStrategy = setLoadingStrategy, + + loadedModuleChanged = loadedModuleChanged.Event, + } +end + +return createModuleLoader diff --git a/src/ModuleLoader.spec.luau b/src/createModuleLoader.spec.luau similarity index 70% rename from src/ModuleLoader.spec.luau rename to src/createModuleLoader.spec.luau index 06ab183..8d73b37 100644 --- a/src/ModuleLoader.spec.luau +++ b/src/createModuleLoader.spec.luau @@ -8,7 +8,8 @@ local beforeEach = JestGlobals.beforeEach local afterEach = JestGlobals.afterEach local describeEach = describe.each :: any -local ModuleLoader = require("./ModuleLoader") +local createModuleLoader = require("./createModuleLoader") +local types = require("./types") local loadModuleEnabled = pcall(function() return debug["loadmodule"](Instance.new("ModuleScript")) @@ -43,56 +44,33 @@ local function createModuleTest(tree: ModuleTestTree, parent: Instance?): any end local mockModuleSource = {} -local loader: ModuleLoader.ModuleLoader +local loader +local moduleRegistry local tree describeEach({ - "loadstring", - "loadmodule", -})("%s", function(loadingStrategy) - if loadingStrategy == "loadmodule" and not loadModuleEnabled then + "LoadString", + "LoadModule", +} :: { types.LoadingStrategy })("%s", function(loadingStrategy) + if loadingStrategy == "LoadModule" and not loadModuleEnabled then test = test.skip :: any end beforeEach(function() - loader = ModuleLoader.new() - loader._forceLoadstring = loadingStrategy == "loadstring" + loader = createModuleLoader() + loader.setLoadingStrategy(loadingStrategy) + + moduleRegistry = loader._getModuleRegistry() end) afterEach(function() - loader:clear() + loader.clear() if tree then tree:Destroy() end end) - describe("_trackChanges", function() - test("creates a Janitor instance if it doesn't exist", function() - local mockModuleInstance = Instance.new("ModuleScript") - mockModuleInstance.Source = "return nil" - - expect(loader._janitors[mockModuleInstance]).toBeUndefined() - - loader:_trackChanges(mockModuleInstance) - - expect(loader._janitors[mockModuleInstance]).toBeDefined() - end) - - test("reuses the same Janitor instance for future calls", function() - local mockModuleInstance = Instance.new("ModuleScript") - mockModuleInstance.Source = "return nil" - - loader:_trackChanges(mockModuleInstance) - - local janitor = loader._janitors[mockModuleInstance] - - loader:_trackChanges(mockModuleInstance) - - expect(loader._janitors[mockModuleInstance]).toBe(janitor) - end) - end) - describe("loadedModuleChanged", function() test("fires when a required module has its ancestry changed", function() local mockModuleInstance = Instance.new("ModuleScript") @@ -111,7 +89,7 @@ describeEach({ end) -- Require the module so that events get setup - loader:require(mockModuleInstance) + loader.require(mockModuleInstance) expect(wasFired).toBe(false) @@ -133,11 +111,12 @@ describeEach({ end) -- Require the module so that events get setup - loader:require(mockModuleInstance) + loader.require(mockModuleInstance) expect(wasFired).toBe(false) mockModuleInstance.Source = "Something different" + task.wait() expect(wasFired).toBe(true) end) @@ -166,9 +145,10 @@ describeEach({ end end) - loader:require(tree.ModuleC) + loader.require(tree.ModuleC) tree.ModuleA.Source = "Changed" + task.wait() expect(count).toBe(3) end) @@ -179,9 +159,9 @@ describeEach({ local mockModuleInstance = Instance.new("ModuleScript") mockModuleInstance.Source = "return nil" - loader:cache(mockModuleInstance, mockModuleSource) + loader.cache(mockModuleInstance, mockModuleSource) - local loadedModule = loader._moduleRegistry.getByInstance(mockModuleInstance) + local loadedModule = moduleRegistry.getByInstance(mockModuleInstance) assert(loadedModule, "loaded module undefined") expect(loadedModule.exports).toBe(mockModuleSource) @@ -193,8 +173,8 @@ describeEach({ local mockModuleInstance = Instance.new("ModuleScript") mockModuleInstance.Source = "return nil" - loader:require(mockModuleInstance) - expect(loader._moduleRegistry.getByInstance(mockModuleInstance)).toBeDefined() + loader.require(mockModuleInstance) + expect(moduleRegistry.getByInstance(mockModuleInstance)).toBeDefined() end) test("keeps track of module dependencies", function() @@ -216,24 +196,24 @@ describeEach({ ]], }) - loader:require(tree.Root) + loader.require(tree.Root) - expect(loader._moduleRegistry.getByInstance(tree.Root)).toMatchObject({ + expect(moduleRegistry.getByInstance(tree.Root)).toMatchObject({ dependencies = { [tree.Module1] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toMatchObject({ + expect(moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toMatchObject({ dependencies = { [tree.Module2] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toMatchObject({ + expect(moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toMatchObject({ dependencies = { [tree.Module3] = true, }, }) - expect(loader._moduleRegistry.getByInstance(tree.Module3 :: ModuleScript)).toMatchObject({ + expect(moduleRegistry.getByInstance(tree.Module3 :: ModuleScript)).toMatchObject({ dependencies = {}, }) end) @@ -257,8 +237,8 @@ describeEach({ ]], }) - local sharedModuleFromConsumer1 = loader:require(tree.Consumer1) - local sharedModuleFromConsumer2 = loader:require(tree.Consumer2) + local sharedModuleFromConsumer1 = loader.require(tree.Consumer1) + local sharedModuleFromConsumer2 = loader.require(tree.Consumer2) expect(sharedModuleFromConsumer1).toBe(sharedModuleFromConsumer2) end) @@ -275,9 +255,9 @@ describeEach({ ]], }) - loader:require(tree.Consumer) + loader.require(tree.Consumer) - local loadedModule = loader._moduleRegistry.getByInstance(tree.SharedModule) + local loadedModule = moduleRegistry.getByInstance(tree.SharedModule) assert(loadedModule, "loaded module undefined") expect(loadedModule.consumers[tree.Consumer]).toBeDefined() end) @@ -298,16 +278,16 @@ describeEach({ ]], }) - loader:require(tree.Consumer1) + loader.require(tree.Consumer1) - local loadedModule = loader._moduleRegistry.getByInstance(tree.SharedModule) + local loadedModule = moduleRegistry.getByInstance(tree.SharedModule) assert(loadedModule, "loaded module undefined") expect(loadedModule.consumers[tree.Consumer1]).toBeDefined() expect(loadedModule.consumers[tree.Consumer2]).toBeUndefined() - loader:require(tree.Consumer2) + loader.require(tree.Consumer2) expect(loadedModule.consumers[tree.Consumer1]).toBeDefined() expect(loadedModule.consumers[tree.Consumer2]).toBeDefined() @@ -324,9 +304,9 @@ describeEach({ ]], }) - loader:require(tree.WriteGlobal) + loader.require(tree.WriteGlobal) - local result = loader:require(tree.ReadGlobal) + local result = loader.require(tree.ReadGlobal) expect(result).toBe(true) end) @@ -343,7 +323,7 @@ describeEach({ ]], }) - local result = loader:require(tree.UseGlobal) + local result = loader.require(tree.UseGlobal) expect(result).toBe(true) end) @@ -357,7 +337,7 @@ describeEach({ tree.Name = "SyntaxError" expect(function() - loader:require(tree.Module) + loader.require(tree.Module) end).toThrow(`SyntaxError.Module:1: Incomplete statement: expected assignment or a function call`) end) @@ -382,9 +362,20 @@ describeEach({ tree.Name = "SyntaxError" expect(function() - loader:require(tree.Consumer) + loader.require(tree.Consumer) end).toThrow(`SyntaxError.Module3:1: Incomplete statement: expected assignment or a function call`) end) + + test("when a module's source changes requiring it again uses the new source", function() + local moduleScript = Instance.new("ModuleScript") + moduleScript.Source = "return true" + + expect(loader.require(moduleScript)).toBeTruthy() + + moduleScript.Source = "return false" + + expect(loader.require(moduleScript)).toBeFalsy() + end) end) describe("clearModule", function() @@ -395,13 +386,13 @@ describeEach({ ]], }) - loader:require(tree.Module) + loader.require(tree.Module) - expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.Module)).toBeDefined() - loader:clearModule(tree.Module) + loader.clearModule(tree.Module) - expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Module)).toBeUndefined() end) test("clears all consumers of a module from the cache", function() @@ -420,18 +411,18 @@ describeEach({ ]], }) - loader:require(tree.Consumer1) - loader:require(tree.Consumer2) + loader.require(tree.Consumer1) + loader.require(tree.Consumer2) - expect(loader._moduleRegistry.getByInstance(tree.Consumer1)).toBeDefined() - expect(loader._moduleRegistry.getByInstance(tree.Consumer2)).toBeDefined() - expect(loader._moduleRegistry.getByInstance(tree.SharedModule)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.Consumer1)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.Consumer2)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.SharedModule)).toBeDefined() - loader:clearModule(tree.SharedModule) + loader.clearModule(tree.SharedModule) - expect(loader._moduleRegistry.getByInstance(tree.Consumer1)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Consumer2)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.SharedModule)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Consumer1)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Consumer2)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.SharedModule)).toBeUndefined() end) test("only clears modules in the consumer chain", function() @@ -448,15 +439,15 @@ describeEach({ ]], }) - loader:require(tree.Consumer) - loader:require(tree.Independent) + loader.require(tree.Consumer) + loader.require(tree.Independent) - expect(#loader._moduleRegistry.getAllModules()).toBe(3) + expect(#moduleRegistry.getAllModules()).toBe(3) - loader:clearModule(tree.Module) + loader.clearModule(tree.Module) - expect(#loader._moduleRegistry.getAllModules()).toBe(1) - expect(loader._moduleRegistry.getByInstance(tree.Independent)).toBeDefined() + expect(#moduleRegistry.getAllModules()).toBe(1) + expect(moduleRegistry.getByInstance(tree.Independent)).toBeDefined() end) test("fires loadedModuleChanged when clearing a module", function() @@ -476,8 +467,8 @@ describeEach({ wasFired = true end) - loader:require(tree.Consumer) - loader:clearModule(tree.Consumer) + loader.require(tree.Consumer) + loader.clearModule(tree.Consumer) expect(wasFired).toBe(true) end) @@ -507,8 +498,8 @@ describeEach({ count += 1 end) - loader:require(tree.Consumer) - loader:clearModule(tree.Module3 :: ModuleScript) + loader.require(tree.Consumer) + loader.clearModule(tree.Module3 :: ModuleScript) expect(count).toBe(4) end) @@ -522,7 +513,7 @@ describeEach({ -- Do nothing if the module hasn't been cached local module = Instance.new("ModuleScript") - loader:clearModule(module) + loader.clearModule(module) expect(wasFired).toBe(false) end) end) @@ -532,13 +523,13 @@ describeEach({ local mockModuleInstance = Instance.new("ModuleScript") mockModuleInstance.Source = "return nil" - loader:cache(mockModuleInstance, mockModuleSource) + loader.cache(mockModuleInstance, mockModuleSource) - expect(#loader._moduleRegistry.getAllModules()).toBe(1) + expect(#moduleRegistry.getAllModules()).toBe(1) - loader:clear() + loader.clear() - expect(#loader._moduleRegistry.getAllModules()).toBe(0) + expect(#moduleRegistry.getAllModules()).toBe(0) end) end) @@ -561,31 +552,31 @@ describeEach({ end) test("removes all consumers of a changed module from the cache", function() - loader:require(tree.ModuleA) + loader.require(tree.ModuleA) - local hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil + local hasItems = next(moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil + hasItems = next(moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(false) end) test("does not interfere with other cached modules", function() - loader:require(tree.ModuleA) - loader:require(tree.ModuleC) + loader.require(tree.ModuleA) + loader.require(tree.ModuleC) - local hasItems = next(loader._moduleRegistry.getAllModules()) ~= nil + local hasItems = next(moduleRegistry.getAllModules()) ~= nil expect(hasItems).toBe(true) tree.ModuleB.Source = 'return "ModuleB Reloaded"' task.wait() - expect(loader._moduleRegistry.getByInstance(tree.ModuleA)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.ModuleB)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.ModuleC)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.ModuleA)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.ModuleB)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.ModuleC)).toBeDefined() end) end) @@ -613,7 +604,7 @@ describeEach({ end) afterEach(function() - loader:clear() + loader.clear() rbxtsInclude:Destroy() end) @@ -645,14 +636,14 @@ describeEach({ ]], }) - loader:require(tree.Root) - loader:clearModule(tree.Shared) + loader.require(tree.Root) + loader.clearModule(tree.Shared) - expect(loader._moduleRegistry.getByInstance(mockRuntime)).toBeDefined() - expect(loader._moduleRegistry.getByInstance(tree.Shared)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Root)).toBeUndefined() + expect(moduleRegistry.getByInstance(mockRuntime)).toBeDefined() + expect(moduleRegistry.getByInstance(tree.Shared)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Module1 :: ModuleScript)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Module2 :: ModuleScript)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Root)).toBeUndefined() end) test("clear() clears the roblox-ts runtime when calling", function() @@ -663,11 +654,11 @@ describeEach({ ]], }) - loader:require(tree.Module) - loader:clear() + loader.require(tree.Module) + loader.clear() - expect(loader._moduleRegistry.getByInstance(mockRuntime)).toBeUndefined() - expect(loader._moduleRegistry.getByInstance(tree.Module)).toBeUndefined() + expect(moduleRegistry.getByInstance(mockRuntime)).toBeUndefined() + expect(moduleRegistry.getByInstance(tree.Module)).toBeUndefined() end) end) end) diff --git a/src/init.luau b/src/init.luau index 4278cfc..b9bb6d8 100644 --- a/src/init.luau +++ b/src/init.luau @@ -1,6 +1,6 @@ local ModuleLoader = require("./ModuleLoader") -export type ModuleLoader = ModuleLoader.ModuleLoader -export type Class = ModuleLoader.ModuleLoader +export type ModuleLoader = ModuleLoader.ModuleLoaderClass +export type Class = ModuleLoader.ModuleLoaderClass return ModuleLoader diff --git a/src/types.luau b/src/types.luau index f9a2526..036ccde 100644 --- a/src/types.luau +++ b/src/types.luau @@ -1,4 +1,6 @@ -export type LoadedModuleExports = unknown +export type LoadingStrategy = "Automatic" | "LoadString" | "LoadModule" + +export type LoadedModuleExports = unknown? export type LoadedModule = { instance: ModuleScript, @@ -17,4 +19,20 @@ export type ModuleRegistry = { getByFullName: (fullName: string) -> LoadedModule?, } +export type LoadModuleFn = (moduleScript: ModuleScript) -> LoadedModuleExports + +export type ModuleLoader = { + _getModuleRegistry: () -> ModuleRegistry, + + require: LoadModuleFn, + loadModule: LoadModuleFn, + cache: (moduleScript: ModuleScript, result: LoadedModuleExports) -> (), + clearModule: (moduleScript: ModuleScript) -> (), + clear: () -> (), + + setLoadingStrategy: (loadingStrategy: LoadingStrategy) -> (), + + loadedModuleChanged: RBXScriptSignal, +} + return nil