search

lodev09 / react-native-true-sheet

The true native bottom sheet experience 💩
https://sheet.lodev09.com
MIT License
394 stars 12 forks source link

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

Closed wcastand closed 2 months ago

wcastand commented 3 months ago

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:

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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

lodev09 commented 2 months ago

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: https://github.com/lodev09/react-native-true-sheet/blob/329026d62e8c077aa1ae55e09d6d57dcc5fd6efd/ios/TrueSheetView.swift#L142-L147

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 commented 2 months ago

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

wcastand commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@wcastand can you try with isFocused?

wcastand commented 2 months ago

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

lodev09 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/expo/expo/issues/27557

lodev09 commented 2 months ago

Yep, worked for me too 👍

wcastand commented 2 months ago

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 commented 2 months ago

yep https://github.com/lodev09/react-native-true-sheet/pull/59

wcastand commented 2 months ago

amazing so fast ahah !!