From 33dec35d88d7d34a8a6f6c41d06cbe4429b64ebf Mon Sep 17 00:00:00 2001 From: jeparlefrancais Date: Tue, 14 Jan 2025 21:26:26 -0500 Subject: [PATCH 1/3] Fix navigation params containing None entries There were cases where using RoactNavigation.None in params were not handled incorrectly, which led to params containing the None symbol. --- src/routers/StackRouter.lua | 4 +- src/routers/SwitchRouter.lua | 4 +- src/routers/_tests_/Routers.roblox.spec.lua | 56 +++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/routers/StackRouter.lua b/src/routers/StackRouter.lua index 4081f99..871fdf3 100644 --- a/src/routers/StackRouter.lua +++ b/src/routers/StackRouter.lua @@ -369,7 +369,7 @@ return function(routeArray, config) routes[lastRouteIndex] = Object.assign(table.clone(route), { params = if action.params == Object.None then Object.None - elseif not route.params then table.clone(action.params) + elseif not route.params then Object.assign({}, action.params) else Object.assign(table.clone(route.params), action.params), }) end @@ -551,7 +551,7 @@ return function(routeArray, config) params = if lastRoute.params and action.params then Object.assign(table.clone(lastRoute.params), action.params) elseif lastRoute.params then table.clone(lastRoute.params) - elseif action.params then table.clone(action.params) + elseif action.params then Object.assign({}, action.params) else {} end local routes = table.clone(state.routes) diff --git a/src/routers/SwitchRouter.lua b/src/routers/SwitchRouter.lua index 647faa4..6a0ec95 100644 --- a/src/routers/SwitchRouter.lua +++ b/src/routers/SwitchRouter.lua @@ -295,7 +295,7 @@ return function(routeArray, config) then Object.None else if newChildState.params then Object.assign(table.clone(newChildState.params), action.params) - else table.clone(action.params), + else Object.assign({}, action.params), }) end @@ -327,7 +327,7 @@ return function(routeArray, config) params = if lastRoute.params and action.params then Object.assign(table.clone(lastRoute.params), action.params) elseif lastRoute.params then table.clone(lastRoute.params) - elseif action.params then table.clone(action.params) + elseif action.params then Object.assign({}, action.params) else {} end local routes = table.clone(state.routes) diff --git a/src/routers/_tests_/Routers.roblox.spec.lua b/src/routers/_tests_/Routers.roblox.spec.lua index 27f450c..59aa2f8 100644 --- a/src/routers/_tests_/Routers.roblox.spec.lua +++ b/src/routers/_tests_/Routers.roblox.spec.lua @@ -55,6 +55,62 @@ for routerName, Router in pairs(ROUTERS) do expect(state1.routes[state1.index].params.foo).toBeNil() end) + it("setParams using RoactNavigation.None does not keep the None value", function() + local state0 = router.getStateForAction( + NavigationActions.setParams({ params = { foo = RoactNavigation.None }, key = initRoute.key }), + initState + ) + + expect(state0.routes[state0.index]).toEqual(expect.objectContaining({ params = {} })) + end) + + it("setParams to RoactNavigation.None does not set any params", function() + local state0 = router.getStateForAction( + NavigationActions.setParams({ params = RoactNavigation.None, key = initRoute.key }), + initState + ) + + expect(state0.routes[state0.index].params).toEqual(nil) + end) + + it("removes params with RoactNavigation.None when navigating to the same route", function() + initState = router.getStateForAction(NavigationActions.init({ + params = { a = 1 } + })) + initRoute = initState.routes[initState.index] + expect(initState.routes[initState.index].params).toEqual(expect.objectContaining({ a = 1 })) + + local state0 = router.getStateForAction( + NavigationActions.navigate({ params = RoactNavigation.None, routeName = initRoute.routeName, key = initRoute.key }), + initState + ) + + expect(state0.routes[state0.index].params).toEqual(nil) + end) + + it("does not leave a specific param to RoactNavigation.None when navigating to the same route", function() + local state0 = router.getStateForAction( + NavigationActions.navigate({ params = { a = RoactNavigation.None }, routeName = initRoute.routeName, key = initRoute.key }), + initState + ) + + expect(state0.routes[state0.index].params).toEqual({}) + end) + + it("removes a specific param with RoactNavigation.None when navigating to the same route", function() + initState = router.getStateForAction(NavigationActions.init({ + params = { a = 1, b = 2 } + })) + initRoute = initState.routes[initState.index] + + local state0 = router.getStateForAction( + NavigationActions.navigate({ params = { a = RoactNavigation.None }, routeName = initRoute.routeName, key = initRoute.key }), + initState + ) + + expect(state0.routes[state0.index].params).toEqual(expect.objectContaining({ b = 2 })) + end) + it("navigate clears individual params using RoactNavigation.None", function() local state0 = router.getStateForAction( NavigationActions.setParams({ params = { foo = 10, bar = 20 }, key = initRoute.key }), From 42ce6101fedd6ca0b206b6fb62517eb513ddd26f Mon Sep 17 00:00:00 2001 From: jeparlefrancais Date: Tue, 14 Jan 2025 21:37:26 -0500 Subject: [PATCH 2/3] fix format --- src/routers/_tests_/Routers.roblox.spec.lua | 22 ++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/routers/_tests_/Routers.roblox.spec.lua b/src/routers/_tests_/Routers.roblox.spec.lua index 59aa2f8..7863adb 100644 --- a/src/routers/_tests_/Routers.roblox.spec.lua +++ b/src/routers/_tests_/Routers.roblox.spec.lua @@ -75,13 +75,17 @@ for routerName, Router in pairs(ROUTERS) do it("removes params with RoactNavigation.None when navigating to the same route", function() initState = router.getStateForAction(NavigationActions.init({ - params = { a = 1 } + params = { a = 1 }, })) initRoute = initState.routes[initState.index] expect(initState.routes[initState.index].params).toEqual(expect.objectContaining({ a = 1 })) local state0 = router.getStateForAction( - NavigationActions.navigate({ params = RoactNavigation.None, routeName = initRoute.routeName, key = initRoute.key }), + NavigationActions.navigate({ + params = RoactNavigation.None, + routeName = initRoute.routeName, + key = initRoute.key, + }), initState ) @@ -90,7 +94,11 @@ for routerName, Router in pairs(ROUTERS) do it("does not leave a specific param to RoactNavigation.None when navigating to the same route", function() local state0 = router.getStateForAction( - NavigationActions.navigate({ params = { a = RoactNavigation.None }, routeName = initRoute.routeName, key = initRoute.key }), + NavigationActions.navigate({ + params = { a = RoactNavigation.None }, + routeName = initRoute.routeName, + key = initRoute.key, + }), initState ) @@ -99,12 +107,16 @@ for routerName, Router in pairs(ROUTERS) do it("removes a specific param with RoactNavigation.None when navigating to the same route", function() initState = router.getStateForAction(NavigationActions.init({ - params = { a = 1, b = 2 } + params = { a = 1, b = 2 }, })) initRoute = initState.routes[initState.index] local state0 = router.getStateForAction( - NavigationActions.navigate({ params = { a = RoactNavigation.None }, routeName = initRoute.routeName, key = initRoute.key }), + NavigationActions.navigate({ + params = { a = RoactNavigation.None }, + routeName = initRoute.routeName, + key = initRoute.key, + }), initState ) From fab5f467321a141e58798222421315a9901318b4 Mon Sep 17 00:00:00 2001 From: jeparlefrancais Date: Tue, 14 Jan 2025 21:37:34 -0500 Subject: [PATCH 3/3] add entry to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e33e6b..033e398 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ___ Work in progress, to be added to next release notes. +* Fixed RoactNavigation.None present in params when navigating ([#3](https://github.com/jsdotlua/roact-navigation/pull/3)) + ### v0.5.10 * Added `useNavigation` hook to the module's export ([#2](https://github.com/jsdotlua/roact-navigation/pull/2))