Skip to content
This repository has been archived by the owner on Dec 3, 2022. It is now read-only.

useNavigationParam: useState-like interface? #19

Open
FireyFly opened this issue May 23, 2019 · 6 comments
Open

useNavigationParam: useState-like interface? #19

FireyFly opened this issue May 23, 2019 · 6 comments

Comments

@FireyFly
Copy link

FireyFly commented May 23, 2019

Is it too late to suggest a change in the interface for useNavigationParam?

I'm working on some code and found myself using the navigation param as the only state for a scene of mine. The idea is that you pass in a value and a callback function as navigation params to a modal scene, which lets you edit said value and eventually accept it (which triggers the callback) or cancel (which just triggers goBack for now, but might as well call another callback if defined).

so I found myself writing something like

const value = navigation.getParam('value', '');
const setValue = (newValue) => { navigation.setParams({ value: newValue }); };

and using it much like I'd use useState.

So, thinking "this would be a useful abstraction on top of a useNavigation hook", I found this repo. I was thinking an interface like this for useNavigationParam might make sense, where you can either destruct out only the value, or get both the value and a setter function to update it. So I figured I might as well create an issue with this suggestion, and see what others think. :)

@satya164
Copy link
Member

and using it much like I'd use useState.

The code you posted doesn't need to be a hook since it doesn't use any hook specific features like containing state, using context, storing refs etc. Naming it like a hook will be confusing if it's not really a hook.

@FireyFly
Copy link
Author

FireyFly commented May 23, 2019

@satya164 well sure, but it would be defined in terms of the useNavigation hook in this repo, much like the current useNavigationParam is. From the readme:

useNavigationParam(paramName)

Access a param for the current navigation state

function MyScreen() {
  const name = useNavigationParam('name');
  return <p>name is {name}</p>;
}

Literally the same as useNavigation().getParam(paramName)

The hook I proposed is a modification of this, and could be implemented along the lines of

const useNavigationParam = (paramName, defaultValue) => [
  useNavigation().getParam(paramName, defaultValue),
  (newValue) => useNavigation().setParams({ [paramName]: newValue }),
];

but the bit I'm worried about is if the ship has sailed with regard to the API of useNavigationParam. (Since my proposed API wouldn't be backwards-compatible here.)

@satya164
Copy link
Member

Makes sense. Agree that useNavigationParam should return a value, setter pair.

@benseitz
Copy link
Contributor

@FireyFly Will you implement this feature or do you want me to open a PR? :)

@satya164 How will you handel the break in backwards compatibility? Since it's an alpha and there is no CHANGELOG yet I fear some users might trip over this.

@FireyFly
Copy link
Author

@benseitz feel free to open a PR! 👍

A simple implementation should be straightforward (something like the above), but I don't really have the time to look at this currently (over this weekend/next week, at least).

@benseitz
Copy link
Contributor

@FireyFly @satya164 I opened a PR #20 but it still has some problems, so I'd appreciate your help :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants