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

[CRASH] IOS still getting a crash even after last patch #39

Closed
wcastand opened this issue Jun 13, 2024 · 21 comments · Fixed by #52
Closed

[CRASH] IOS still getting a crash even after last patch #39

wcastand opened this issue Jun 13, 2024 · 21 comments · Fixed by #52

Comments

@wcastand
Copy link

wcastand commented Jun 13, 2024

Still getting a crash on reloading in dev on the expo app. i understand if you can't reproduce you can't fix it. just thought worth having an open issue if it happens to other people.

Seems to occur after a reload in dev mode (didn't build a production app yet to be 100% sure it's not happening in production app code though)

Expo: "^51.0.13"
true sheet: "^0.12.0"

Step to reproduce:

  • open the app in dev mode
  • hit R to reload the app
  • navigate to a page with a TrueSheet in it (i don't need to try to open the sheet or anything)

screenshot of xcode crash if that can help
Screenshot 2024-06-13 at 15 52 27
Screenshot 2024-06-13 at 15 52 38
Screenshot 2024-06-13 at 15 57 30
Screenshot 2024-06-13 at 15 57 38
Screenshot 2024-06-13 at 15 57 47
Screenshot 2024-06-13 at 15 57 54

@lodev09
Copy link
Owner

lodev09 commented Jun 13, 2024

Hey @wcastand, thanks for reporting.

Are you constantly experiencing this? Would be nice to have a reproducible example, or maybe a gist of the related code.

@wcastand
Copy link
Author

wcastand commented Jun 13, 2024

yes it's consistent.

i will try a reproducible example but guessing it's related to my full setup and it's not a small one and i can't really share it since it's private repo.

i'll try my best for the gist. the screen where i manage to make it crash consistently on reload is my sign-in screen so i will give the best info and code possible.

using expo router with this arch:

- app
  _layout.tsx
  - (app)
    _layout.tsx
    sign-in.tsx

in the sign-in screen, the true sheet is part of the input as an optional hint.
the input in sign-in is not showing any hint so no trueSheet.

I have a SafeAreaView(in _layout.tsx) and It's inside a ScrollView(in Layout.tsx)

// sign-in screen
  return (
    <Layout testID="signin-screen">
      <Stack.Screen options={{ title: t("HEADER_TITLE") }} />
      <KeyboardAvoidMarginView>
        <YStack flex={1} gap="$base">
          <Heading as="h3">{t("TITLE")}</Heading>
          <Input
            autoFocus
            name="email"
            control={control}
            label={t("EMAIL")}
            autoCapitalize={"none"}
            testID="signin-email-input"
            keyboardType="email-address"
            placeholder={t("PLACEHOLDER_EMAIL")}
          />
        </YStack>
        <YStack rowGap="$sm">
          {showSignUpButton && (
            <Button type="secondary" onPress={redirectToSignUp}>
              <Button.Text>{t("CREATE_PROFILE")}</Button.Text>
            </Button>
          )}
          <Button testID="signin-btn" onPress={handleSubmit(onSubmit)} disabled={isSubmitting}>
            <Button.Text>{t("RECEIVE_CODE")}</Button.Text>
          </Button>
        </YStack>
      </KeyboardAvoidMarginView>
    </Layout>
  )

the Input is a Controlled Input with react-hook-form

// Input.tsx
  return (
    <>
      <Controller
        name={name}
        control={control}
        render={({ field: { onChange, onBlur, value }, fieldState: { error } }) => {
          const color = disabled ? "$neutral400" : error ? "$error400" : "$neutral900"
          const borderColor = disabled ? "$transparent" : error ? "$error400" : "$transparent"
          return (
            <YStack gap="$xsm" {...containerProps} opacity={disabled ? 0.4 : 1}>
              {(label || info || hint) && (
                <XStack jc={label ? "space-between" : "flex-end"} ai="flex-end">
                  {label && (
                    <YStack>
                      <Body fontWeight="$bold" color={color}>
                        {label}
                      </Body>
                      {sublabel && (
                        <Body as="body2" color={color}>
                          {sublabel}
                        </Body>
                      )}
                    </YStack>
                  )}
                  {info ? (
                    <Link type={error ? "red" : "primary"} onPress={openSheet}>
                      <Info />
                    </Link>
                  ) : (
                    hint && (
                      <Body as="body2" color={color}>
                        {hint}
                      </Body>
                    )
                  )}
                </XStack>
              )}
              <InputBase
                ref={myRef}
                onChangeText={(v) => onChange(transformOnChange ? transformOnChange(v) : v)}
                onBlur={onBlur}
                editable={!disabled}
                value={value}
                borderColor={borderColor}
                focusStyle={{ borderColor: color }}
                {...props}
              />
              {!hideErrorContainer && error && (
                <YStack position="relative" bg="white">
                  <Body as="body2" color="$error400">
                    {error?.message}
                  </Body>
                </YStack>
              )}
            </YStack>
          )
        }}
      />

      {info && (
        <BottomSheet name={`bottom_sheet_field_${name}`} ref={trueSheetRef}>
          <View gap="$base">{info}</View>
        </BottomSheet>
      )}
    </>
  )

i tried to remove the conditional, or add it inside the bottomSheet instead of the full bottom sheet. but still getting consistent crash.

i get no crash on the welcome screen which has a TrueSheet too, so my best guess is that it's somehow related to the way it is in the Input.

I remove all the flex:1 in the codebase inside the TrueSheet.

@lodev09
Copy link
Owner

lodev09 commented Jun 13, 2024

Ah.. the conditional rendering might be causing it.

{info && (
  <BottomSheet name={`bottom_sheet_field_${name}`} ref={trueSheetRef}>
    <View gap="$base">{info}</View>
  </BottomSheet>
)}

Try it like this:

<BottomSheet name={`bottom_sheet_field_${name}`} ref={trueSheetRef}>
  {info && <View gap="$base">{info}</View>}
</BottomSheet>

Alternatively, you could render a "placeholder" if you're worried about size changes on the fly.

I'm using TrueSheet on my project as a Select field as well and I don't see this issue happening. Rendering the sheet in the background is fine since it's handled natively.

@wcastand
Copy link
Author

i already tried that and was still getting the crash :/

i will give it a second try just in case tho. i let you know if it works this time

@wcastand
Copy link
Author

wcastand commented Jun 14, 2024

ok so rebuilt the app with cleared cache and all to be safe this time when i test. so far could not reproduce the bug. will play with it a little bit longer but maybe it's solved.

the conditional seems to solve the problem, I will see if anywhere in my app i still get crashes or not but the place where i could reproduce it everytime is not crashing anymore so good sign.

thanks for the help!

@wcastand
Copy link
Author

wcastand commented Jul 1, 2024

still getting some crash in the production app this time. mainly happens after an update is received and the app is reloaded, when i visit a screen with a true sheet, the app crashes.

Same as before, no interaction with the sheet, just i open the screen with the sheet in it and it crashes instantly.

can find the report of the crash

Screenshot 2024-07-01 at 10 30 44

@wcastand wcastand reopened this Jul 1, 2024
@lodev09
Copy link
Owner

lodev09 commented Jul 1, 2024

Yeah, there's some weird stuff going on with expo-updates. Reloading after update is basically the same as in dev mode hot reload. I'm not sure what's the proper fix for it since it's very hard to reproduce ;(

In this block:

func viewControllerDidChangeWidth(_ width: CGFloat) {
guard let containerView else { return }
let size = CGSize(width: width, height: containerView.bounds.height)
bridge?.uiManager.setSize(size, for: containerView)
}

Can you change it with this:

  func viewControllerDidChangeWidth(_ width: CGFloat) {
    guard let containerView, let uiManager = bridge?.uiManager else { return }

    let size = CGSize(width: width, height: containerView.bounds.height)
    uiManager.setSize(size, for: containerView)
  }

See if that fixes the problem.

@lodev09
Copy link
Owner

lodev09 commented Jul 1, 2024

@wcastand PR #52. Try it and let me know if that fixes the issue

@wcastand
Copy link
Author

wcastand commented Jul 1, 2024

can try to reproduce on dev app, but same as you super hard to reproduce and can't really make a prod version with a github branch :/

@wcastand
Copy link
Author

wcastand commented Jul 3, 2024

so far since the update, it looks like we get no more crash on production app after an update reload. so looks like it did the job. thanks

we still get warning on other things, mostly when we want to open a sheet right after we nagivate to the screen, we get a warning saying it's not intanciated yet even if we check the ref before calling. if i can manage to make a repro i will make a ticket.

@lodev09
Copy link
Owner

lodev09 commented Jul 3, 2024

we still get warning on other things, mostly when we want to open a sheet right after we nagivate to the screen, we get a warning saying it's not intanciated yet even if we check the ref before calling. if i can manage to make a repro i will make a ticket.

Ah, that's pretty normal. You can use the initialIndex instead if you want to present the sheet right after navigating.

@wcastand
Copy link
Author

wcastand commented Jul 4, 2024

i have a weird behavior when i try to use initialIndex to open the sheet when navigating to the screen

look like if i use initialIndexAnimated i can prevent the weird animation of mounting. but would be nice to be able to get the normal present animation on mount too if possible.

want me to open an other issue for it?
https://github.com/lodev09/react-native-true-sheet/assets/2178244/91765a56-9eb6-4a8d-a3e6-e534c19e1c2b

@lodev09
Copy link
Owner

lodev09 commented Jul 4, 2024

@wcastand can you try with isFocused?

@wcastand
Copy link
Author

wcastand commented Jul 4, 2024

what do you mean by isFocused?
is this a props not documented? can't find anything in the docs

@lodev09
Copy link
Owner

lodev09 commented Jul 4, 2024

I think the reason why you're getting that weird issue is that the screen is not yet "fully" rendered. You could try react-navigation useIsFocused hook to render the sheet when the screen is focused.

e.g.

isFocused = useIsFocused()

return (
  {isFocused && (
    <TrueSheet initialIndex={0}>...</TrueSheet>
  )}
)

@lodev09
Copy link
Owner

lodev09 commented Jul 4, 2024

okay I tried useIsFocused and it looks like it's still happening. What's interesting is that it works fine on modal screens.

Unfortunately, this has something to do with react-native-screen, package used by react-navigation, so I don't have control over this issue, natively. 😢

What you can do to fix it is use react-navigation's transistionEnd lifecycle event. This will trigger when the screen is fully shown

@wcastand
Copy link
Author

wcastand commented Jul 4, 2024

a simple hook to get what we need (it's working for us):

import { useNavigation } from "expo-router"
import { useEffect, useState } from "react"

export const useIsScreenFinishMounting = () => {
	const navigation = useNavigation()
	const [state, setState] = useState({
		opening: false,
		closing: false,
		done: false,
	})
	useEffect(() => {
		const unsubscribe = navigation.addListener("transitionEnd", (e) => {
			setState({
				opening: !e?.data?.closing ?? true,
				closing: e?.data?.closing ?? false,
				done: true,
			})
		})

		return unsubscribe
	}, [navigation])

	return state
}

altho somehow i don't get proper typings on it, maybe related to coming from expo-router

i guess we won't get them since expo close every issue even valid ones
expo/expo#27557

@lodev09
Copy link
Owner

lodev09 commented Jul 4, 2024

Yep, worked for me too 👍

@wcastand
Copy link
Author

wcastand commented Jul 4, 2024

also kind of related but because we use sheet as boolean (open/close) and not multiple size, i was looking for a open props or something to open on mount. never thought about initialIndex for it. if there is a FAQ or guide section, would be a good idea to mention and put that info out there.

like
How to open the sheet on mount ?
and a little guide to use initialIndex and what we talked about above maybe?

do you have a place for this kind of stuff on the site/repo?

@lodev09
Copy link
Owner

lodev09 commented Jul 4, 2024

yep #59

@wcastand
Copy link
Author

wcastand commented Jul 4, 2024

amazing so fast ahah !!

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 a pull request may close this issue.

2 participants