这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@joshuayoes
Copy link
Contributor

@joshuayoes joshuayoes commented Apr 21, 2023

Please verify the following:

  • yarn test jest tests pass with new tests, if relevant
  • README.md has been updated with your changes, if relevant

Describe your PR

This PR strongly types our navigationRef with our actual app stack types, and updates the navigate utility function to have the correct type.

This also addresses the typescript 5.1 error described in this PR: #2393

Additionally, this PR also removes explicit any type usages for Ignite 🥳

@joshuayoes joshuayoes changed the title Strongly type navigationRef, fix navigate argument type Strongly type navigationRef, fix navigate() utility args type Apr 21, 2023
Copy link
Contributor

@frankcalise frankcalise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! This will be a better experience out of the box for those toggling strict mode too. Probably some more work to be done for that but a great start

const [isRestored, setIsRestored] = useState(initNavState)

const routeNameRef = useRef<string | undefined>()
const routeNameRef = useRef<keyof AppStackParamList | undefined>()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there were other navigators involved would you union those param lists here as well? For example the ones from the Demo Navigator in the case of the demo boilerplate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we'd want to include that here, since we are referencing the root navigator being passed around.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we document that then so it could incorporate future nested Stacks etc as params here for the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed up a commit to add some comments to this: dbce8f4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frankcalise you good with the comment on this? Then I'll be good to merge

@joshuayoes joshuayoes force-pushed the fix/navigation-types branch from dbce8f4 to 18032cd Compare April 24, 2023 20:22
@frankcalise
Copy link
Contributor

frankcalise commented Apr 25, 2023 via email

@joshuayoes joshuayoes changed the title Strongly type navigationRef, fix navigate() utility args type Strongly type navigationRef, update navigate() utility args type Apr 25, 2023
@joshuayoes joshuayoes merged commit 2dad32d into master Apr 25, 2023
@joshuayoes joshuayoes deleted the fix/navigation-types branch April 25, 2023 23:38
@infinitered-circleci
Copy link
Collaborator

🎉 This PR is included in version 8.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants