Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ENG-9886): deprecated false positive #859

Merged
merged 1 commit into from
Oct 6, 2023
Merged

fix(ENG-9886): deprecated false positive #859

merged 1 commit into from
Oct 6, 2023

Conversation

marklawlor
Copy link
Contributor

Motivation

Fix ENG-9886.

Execution

useDeprecated is a hook I added a while ago to easily log deprecated warnings once, and handle edge cases such as preventing multiple warnings during SSR & CI environment. I added code comments to explain its parameters.

The issue was the guard has a default value of true. So if you pass undefined, instead of using undefined it will switch to true. This is counter intuitive and a sign this hook needs to be refactored.

I like the guard having a truthy default, because you can simply write useDeprecated(<message>) with no guard. Maybe we need to split it into two functions?

useDeprecated(<message>, <key>)
useDeprecatedWithGuard(<message>, <guard>, <key>)

@linear
Copy link

linear bot commented Aug 28, 2023

ENG-9886 Using useSearchParams will warn with the wrong message

The following warning is thrown:

The `redirect` prop on <Screen /> is deprecated and will be removed. Please use `router.redirect` instead 

Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

Include some form of test plan in the PR description or any indication that this code was run and verified as working.

@nabilfreeman
Copy link

nabilfreeman commented Sep 25, 2023

Any updates here? This is the cause of the false positive warning

The `redirect` prop on <Screen /> is deprecated and will be removed. Please use `router.redirect` instead

that is bugging a lot of people on the latest Expo Router (2.0.8) and latest Expo SDK (49)

@EvanBacon EvanBacon merged commit 9f8a67b into main Oct 6, 2023
6 checks passed
@EvanBacon EvanBacon deleted the ENG-9886 branch October 6, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants