Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: software-mansion/react-native-screens
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 3.33.0
Choose a base ref
...
head repository: software-mansion/react-native-screens
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3.34.0
Choose a head ref
  • 4 commits
  • 7 files changed
  • 7 contributors

Commits on Aug 5, 2024

  1. fix(CI): extend logging in architecture-integrity scripts & add Nativ…

    …eProxy.kt to blacklist (#2281)
    
    ## Description
    
    I was initially confused why the CI fails, so I debugged it a bit and it
    turned out that we forgot to add `NativeProxy.kt` to blacklist in the
    integrity scripts. <- Did it.
    
    Because of the initial confusion I've decided to add some more logging &
    error handling mechanisms.
    
    ## Test code and steps to reproduce
    
    CI now passes. If similar case happens in the future we will get a nice
    error message.
    
    ## Checklist
    
    - [x] Ensured that CI passes
    kkafar authored Aug 5, 2024

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy the full SHA
    3097f01 View commit details
  2. chore(deps): aggregate updates from dependabot (#2267)

    ## Description
    
    * #2266
    * #2272
    
    
    ## Changes
    
    <!--
    Please describe things you've changed here, make a **high level**
    overview, if change is simple you can omit this section.
    
    For example:
    
    - Updated `about.md` docs
    
    -->
    
    <!--
    
    ## Screenshots / GIFs
    
    Here you can add screenshots / GIFs documenting your change.
    
    You can add before / after section if you're changing some behavior.
    
    ### Before
    
    ### After
    
    -->
    
    ## Test code and steps to reproduce
    
    <!--
    Please include code that can be used to test this change and short
    description how this example should work.
    This snippet should be as minimal as possible and ready to be pasted
    into editor (don't exclude exports or remove "not important" parts of
    reproduction example)
    -->
    
    ## Checklist
    
    - [ ] Included code example that can be used to test this change
    - [ ] Updated TS types
    - [ ] Updated documentation: <!-- For adding new props to native-stack
    -->
    - [ ]
    https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
    - [ ]
    https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
    - [ ]
    https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
    - [ ]
    https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
    - [ ] Ensured that CI passes
    
    ---------
    
    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Janic Duplessis <janicduplessis@gmail.com>
    Co-authored-by: Alex Duży <91994767+alduzy@users.noreply.github.com>
    Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
    Co-authored-by: Wojciech Lewicki <wojciech.lewicki@swmansion.com>
    6 people authored Aug 5, 2024

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy the full SHA
    032ec01 View commit details
  3. fix(Android,Fabric,bridge-mode): patch crash with context detached fr…

    …om activity (#2276)
    
    ## Description
    
    When running on Fabric + "bridgefull" combination, react packages are
    initialised very early in application lifetime (much earlier than in
    Fabric + bridgeless), when there is no activity injected to
    `ReactContext` yet. Thus when creating packages they do not have access
    to the activity and we relied on this access up to this moment to setup
    `ScreenDummyLayoutHelper` object, throwing exception if activity was not
    available.
    
    ## Changes
    
    This diff delays initialisation of the dummy layout in case there is no
    access to activity when creating the object up to the point of
    invocation of `onHostResume` callbacks of `ReactContext` (this is the
    very moment activity is injected into the context), avoiding the crash.
    There is also fallback mechanism added in `computeDummyLayout` method,
    so that when accessed from C++ layer it has another chance of
    initialising the dummy layout, before performing computations. If it
    fails (see below for reasons why it might fail) `0f` is returned as
    header height causing content jump, but not crashing the application.
    
    > [!important]
    **Most likely** there is a race condition in this code. `onHostResume`
    is called from UI thread at the execution point, when JS thread & first
    native render & thus commit & layout is already in progress on
    background thread. In case JS thread proceeds to layout &
    `computeDummyLayout` is called from our Screen component descriptor
    before `onHostResume` is called on UI thread & the dummy layout is
    initialised, we hit the case when computing header height will not be
    possible due to uninitialised dummy layout.
    I failed to trigger this behaviour even once for ~30 min of testing with
    trying to put UI thread to sleep etc., however I've also failed to find
    a proof that it won't happen because of some synchronisation / execution
    order.
    
    What's also important is that there is no good way to synchronise these
    threads, because it is not the matter of exclusive access to some
    critical section, but rather a order of access between UI & background
    thread. Some barrier mechanism would be required here, however we do not
    have thread references accessible from our code.
    
    > [!note] 
    One possible solution would be to synchronise access to
    `maybeInitDummyLayoutWithHeader` method & in case the background thread
    hits it before UI, we could wait for UI in some kind of loop ("slow
    spinlock"). This could guarantee correctness, however it is crazy bad,
    because we would impede whole render process for possibly long time.
    Despite the flaws of this approach this might be something to consider
    for the future.
    
    > [!caution]
    One more thing to note is that I rely on [JVM atomicity
    guarantees](https://docs.oracle.com/javase/tutorial/essential/concurrency/atomic.html)
    when reading / writing `isDummyLayoutInitialized` variable. There is
    danger that both threads hit the layout initialisation code at the same
    time and thus leading to corruption. TODO: try to handle this case more
    gracefully. A lock for initialisation code should be enough.
    
    ## Test code and steps to reproduce
    
    Run `FabricExample` with `load(bridgelessEnabled = false)` set in
    `MainApplication` and see that it now works.
    
    ## Checklist
    
    - [x] Ensured that CI passes
    kkafar authored Aug 5, 2024

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy the full SHA
    1713e8f View commit details
  4. Release 3.34.0

    kkafar committed Aug 5, 2024

    Verified

    This commit was signed with the committer’s verified signature.
    kkafar Kacper Kafara
    Copy the full SHA
    505a85b View commit details
Loading