LUAFDN-250 - Add types for connect and dispatch prop (#68)

This PR adds types for the dispatch prop and connect. This includes a standard action dispatch and a thunkful action dispatch variant.

Note that I used the thunkful action dispatch as the default for connect(), which may or may not be desirable, but felt like a better default to me.

Inspired by: https://github.com/reduxjs/react-redux/blob/master/src/types.ts

This RP also adds typechecking to CI with luau-lsp.

.github/workflows/ci.yml

@@ -10,6 +10,32 @@ on:
     - master
 
 jobs:
+  analyze:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        submodules: true
+
+    - name: Install foreman tools
+      uses: Roblox/setup-foreman@v1
+      with:
+        version: "^1.0.1"
+        token: ${{ secrets.GITHUB_TOKEN }}
+
+    - name: Download global Roblox types
+      shell: bash
+      run: curl -s -O https://raw.githubusercontent.com/JohnnyMorganz/luau-lsp/master/scripts/globalTypes.d.lua
+
+    - name: Generate sourcemap for LSP
+      shell: bash
+      run: rojo sourcemap default.project.json -o sourcemap.json
+
+    - name: Analyze
+      shell: bash
+      run: luau-lsp analyze --sourcemap=sourcemap.json --defs=globalTypes.d.lua --defs=testez.d.lua src/
+
   test:
     runs-on: ubuntu-latest
 
@@ -30,11 +56,6 @@ jobs:
         luarocks install luacov
         luarocks install luacov-reporter-lcov
 
-    - name: Test
-      run: |
-        lua -lluacov test/lemur.lua
-        luacov -r lcov
-
     - name: install code quality tools
       uses: Roblox/setup-foreman@v1
       with:
@@ -47,6 +68,16 @@ jobs:
         selene src
         stylua -c src/
 
+    - name: install and run darklua
+      run: |
+        cargo install --git https://gitlab.com/seaofvoices/darklua.git#v0.6.0
+        darklua process src/ src/ --format retain-lines
+
+    - name: Test
+      run: |
+        lua -lluacov test/lemur.lua
+        luacov -r lcov
+
     - name: Report to Coveralls
       uses: coverallsapp/[email protected]
       with:

.gitignore

@@ -13,3 +13,6 @@
 # Rotriever
 rotriever.lock
 Packages
+
+# Misc OS
+.DS_Store

default.project.json

@@ -1,6 +1,23 @@
 {
   "name": "RoactRodux",
   "tree": {
-    "$path": "src"
+    "$className": "Folder",
+    "Roact": {
+      "$path": "modules/roact/src",
+      ".luaurc": {
+          "$className": "StringValue",
+          "$path": "modules/.luaurc"
+      }
+    },
+    "Rodux": {
+      "$path": "modules/rodux/src",
+      ".luaurc": {
+          "$className": "StringValue",
+          "$path": "modules/.luaurc"
+      }
+    },
+    "RoactRodux": {
+      "$path": "src"
+    }
   }
-}
+}

foreman.toml

@@ -2,3 +2,4 @@
 rojo = { source = "rojo-rbx/rojo", version = "=7.2.1" }
 selene = { source = "Kampfkarren/selene", version = "=0.21.1" }
 stylua = { source = "JohnnyMorganz/StyLua", version = "=0.15.1" }
+luau-lsp = { source = "JohnnyMorganz/luau-lsp", version = "=1.8.1" }

modules/.luaurc

@@ -0,0 +1,5 @@
+{
+	"languageMode": "nocheck",
+	"lint": { "*": false },
+	"lintErrors": false
+}

rotriever.toml

@@ -8,4 +8,4 @@ files = ["*", "!*.spec.lua"]
 
 [dependencies]
 Roact = "github.com/roblox/[email protected]"
-Rodux = "github.com/roblox/rodux@3.0"
+Rodux = "github.com/roblox/rodux@4.0.0-rc.0"

src/Symbol.spec.lua

@@ -11,7 +11,7 @@ return function()
 		it("should coerce to the given name", function()
 			local symbol = Symbol.named("foo")
 
-			expect(tostring(symbol):find("foo")).to.be.ok()
+			expect((tostring(symbol):find("foo"))).to.be.ok()
 		end)
 
 		it("should be unique when constructed", function()

src/connect.lua

@@ -1,8 +1,26 @@
+--!strict
 local Roact = require(script.Parent.Parent.Roact)
+
 local shallowEqual = require(script.Parent.shallowEqual)
 local join = require(script.Parent.join)
 local StoreContext = require(script.Parent.StoreContext)
 
+local types = require(script.Parent.types)
+
+type ThunkfulDispatchProp<State> = types.ThunkfulDispatchProp<State>
+type MapStateToProps<StoreState, Props, PartialProps> = types.MapStateToProps<StoreState, Props, PartialProps>
+type MapStateToPropsOrThunk<StoreState, Props, PartialProps> = types.MapStateToPropsOrThunk<
+	StoreState,
+	Props,
+	PartialProps
+>
+type ActionCreatorMap = types.ActionCreatorMap
+type MapDispatchToProps<StoreState, PartialProps> = types.MapDispatchToProps<StoreState, PartialProps>
+type MapDispatchToPropsOrActionCreator<StoreState, PartialProps> = types.MapDispatchToPropsOrActionCreator<
+	StoreState,
+	PartialProps
+>
+
 --[[
 	Formats a multi-line message with printf-style placeholders.
 ]]
@@ -47,7 +65,17 @@ end
 		() -> (storeState, props) -> partialProps
 	mapDispatchToProps: (dispatch) -> partialProps
 ]]
-local function connect(mapStateToPropsOrThunk, mapDispatchToProps)
+local function connect<StoreState, Props, MappedStatePartialProps, MappedDispatchPartialProps>(
+	mapStateToPropsOrThunk: MapStateToPropsOrThunk<
+		StoreState,
+		Props,
+		MappedStatePartialProps
+	>?,
+	mapDispatchToProps: MapDispatchToPropsOrActionCreator<
+		StoreState,
+		MappedDispatchPartialProps
+	>?
+)
 	if mapStateToPropsOrThunk ~= nil then
 		assert(typeof(mapStateToPropsOrThunk) == "function", "mapStateToProps must be a function or nil!")
 	else
@@ -86,6 +114,7 @@ local function connect(mapStateToPropsOrThunk, mapDispatchToProps)
 			if prevState.stateUpdater ~= nil then
 				return prevState.stateUpdater(nextProps.innerProps, prevState)
 			end
+			return nil
 		end
 
 		function Connection:init(props)
@@ -105,7 +134,8 @@ local function connect(mapStateToPropsOrThunk, mapDispatchToProps)
 
 			local storeState = self.store:getState()
 
-			local mapStateToProps = mapStateToPropsOrThunk
+			local mapStateToProps =
+				mapStateToPropsOrThunk :: MapStateToProps<StoreState, Props, MappedStatePartialProps>
 			local mappedStoreState = mapStateToProps(storeState, self.props.innerProps)
 
 			-- mapStateToPropsOrThunk can return a function instead of a state
@@ -133,19 +163,21 @@ local function connect(mapStateToPropsOrThunk, mapDispatchToProps)
 				return self.store:dispatch(...)
 			end
 
-			local mappedStoreDispatch
+			local mappedStoreDispatch: any
 			if mapDispatchType == "table" then
 				mappedStoreDispatch = {}
 
-				for key, actionCreator in pairs(mapDispatchToProps) do
+				for key, actionCreator in pairs(mapDispatchToProps :: ActionCreatorMap) do
 					assert(typeof(actionCreator) == "function", "mapDispatchToProps must contain function values")
 
 					mappedStoreDispatch[key] = function(...)
 						dispatch(actionCreator(...))
 					end
 				end
 			elseif mapDispatchType == "function" then
-				mappedStoreDispatch = mapDispatchToProps(dispatch)
+				mappedStoreDispatch = (mapDispatchToProps :: MapDispatchToProps<StoreState, MappedDispatchPartialProps>)(
+					(dispatch :: any) :: ThunkfulDispatchProp<StoreState>
+				)
 			end
 
 			local stateUpdater = makeStateUpdater(self.store)

src/connect.spec.lua

@@ -6,6 +6,8 @@ return function()
 	local Roact = require(script.Parent.Parent.Roact)
 	local Rodux = require(script.Parent.Parent.Rodux)
 
+	type AnyActionCreator = Rodux.ActionCreator<any, any>
+
 	local function noop()
 		return nil
 	end
@@ -47,7 +49,7 @@ return function()
 
 		it("should accept one table of action creators", function()
 			connect(nil, {
-				foo = function() end,
+				foo = (function() end :: any) :: AnyActionCreator,
 			})
 		end)
 
@@ -219,11 +221,11 @@ return function()
 
 	it("should dispatch the action using a table of action creators", function()
 		local mapDispatchToProps = {
-			increment = function()
+			increment = (function()
 				return {
 					type = "increment",
 				}
-			end,
+			end :: any) :: AnyActionCreator,
 		}
 
 		local function SomeComponent(props)

src/init.lua

@@ -1,6 +1,11 @@
+--!strict
 local StoreProvider = require(script.StoreProvider)
 local StoreContext = require(script.StoreContext)
 local connect = require(script.connect)
+local types = require(script.types)
+
+export type DispatchProp = types.DispatchProp
+export type ThunkfulDispatchProp<State = any> = types.ThunkfulDispatchProp<State>
 
 return {
 	StoreProvider = StoreProvider,

src/types.lua

@@ -0,0 +1,30 @@
+--!strict
+local Rodux = require(script.Parent.Parent.Rodux)
+
+type Action = Rodux.Action<string>
+type ThunkAction<ReturnType, State> = Rodux.ThunkAction<ReturnType, State>
+type ActionCreator<Type, Action, Args...> = Rodux.ActionCreator<Type, Action, Args...>
+
+export type DispatchProp = <Payload>(action: Payload & Action) -> ()
+
+export type ThunkfulDispatchProp<State = any> =
+	DispatchProp
+	& <ReturnType>(thunkAction: ThunkAction<ReturnType, State>) -> ReturnType
+
+export type MapStateToProps<StoreState, Props, PartialProps> = (StoreState, Props) -> PartialProps?
+
+export type MapStateToPropsOrThunk<StoreState, Props, PartialProps> =
+	MapStateToProps<StoreState, Props, PartialProps>
+	| () -> MapStateToProps<StoreState, Props, PartialProps>
+
+export type ActionCreatorMap = {
+	[string]: ActionCreator<any, any, ...any>,
+}
+
+export type MapDispatchToProps<StoreState, PartialProps> = (ThunkfulDispatchProp<StoreState>) -> PartialProps?
+
+export type MapDispatchToPropsOrActionCreator<StoreState, PartialProps> =
+	MapDispatchToProps<StoreState, PartialProps>
+	| ActionCreatorMap
+
+return nil

testez.d.lua

@@ -0,0 +1,24 @@
+declare function afterAll(callback: () -> ()): ()
+declare function afterEach(callback: () -> ()): ()
+
+declare function beforeAll(callback: () -> ()): ()
+declare function beforeEach(callback: () -> ()): ()
+
+declare function describe(phrase: string, callback: () -> ()): ()
+declare function describeFOCUS(phrase: string, callback: () -> ()): ()
+declare function fdescribe(phrase: string, callback: () -> ()): ()
+declare function describeSKIP(phrase: string, callback: () -> ()): ()
+declare function xdescribe(phrase: string, callback: () -> ()): ()
+
+declare function expect(value: any): any
+
+declare function FIXME(optionalMessage: string?): ()
+declare function FOCUS(): ()
+declare function SKIP(): ()
+
+declare function it(phrase: string, callback: () -> ()): ()
+declare function itFOCUS(phrase: string, callback: () -> ()): ()
+declare function fit(phrase: string, callback: () -> ()): ()
+declare function itSKIP(phrase: string, callback: () -> ()): ()
+declare function xit(phrase: string, callback: () -> ()): ()
+declare function itFIXME(phrase: string, callback: () -> ()): ()