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

property is not configurable when accessing props from worklet callback #6134

Closed
kirillzyusko opened this issue Jun 18, 2024 · 8 comments
Closed
Labels
Missing info The user didn't precise the problem enough Missing repro This issue need minimum repro scenario Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS

Comments

@kirillzyusko
Copy link
Contributor

Description

Hey 👋

It seems like after 3.12 there was some breaking changes, so the code like:

import { forwardRef, useCallback } from "react";
import { useAnimatedReaction, useSharedValue } from "react-native-reanimated";

import type React from "react";
import type { ScrollView } from "react-native";

const CustomScrollView = forwardRef<ScrollView, React.PropsWithChildren<{}>>(
  ({ ...rest }, ref) => {
    const input = useSharedValue({ layout: {}, target: 1 });

    const a = useCallback(() => {
      "worklet";

      console.log(rest.snapToOffsets);
    }, [rest.snapToOffsets]);

    useAnimatedReaction(
      () => input.value,
      (current, previous) => {
        a();
      },
      [],
    );

    return null;
  },
);

export default CustomScrollView;

will throw an exception property is not configurable immediately 😕

I've seen similar issues with useAnimatedStyle hook and the suggestion was to not include static styles into useAnimatedStyle hook. Though my problem is a bit different and I don't see a straightforward solution for that.

Steps to reproduce

  1. Copy & paste code from description into example screen
  2. run example app and go to broken screen

Snack or a link to a repository

Reanimated version

3.12.0

React Native version

0.74.0

Platforms

Android, iOS

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Paper (Old Architecture)

Build type

Debug app & dev bundle

Device

iOS simulator

Device model

iPhone 15 Pro

Acknowledgements

Yes

Copy link

Hey! 👋

It looks like you've omitted a few important sections from the issue template.

Please complete Snack or a link to a repository section.

@github-actions github-actions bot added Missing info The user didn't precise the problem enough Missing repro This issue need minimum repro scenario labels Jun 18, 2024
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@github-actions github-actions bot added Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS labels Jun 18, 2024
@kirillzyusko
Copy link
Contributor Author

kirillzyusko commented Jun 18, 2024

@tjzel just discovered that it looks like it has been fixed in #6098 right?

May I kindly ask you to prepare a new patch release for 3.12? (3.12.1 I guess) 🙏 I think it's a very critical one 😓

I see that fix was published under 3.13 RC, but I think this issue may affect many projects, that's why I'm asking to prepare a hotfix release. I hope it makes sense what I'm saying 😅

@tjzel
Copy link
Contributor

tjzel commented Jun 19, 2024

Yeah, I think that's a reasonable request. We released it in an RC version because I'm not absolutely certain it works in all scenarios. Have you tried the RC release to see if it works for you?

@tjzel
Copy link
Contributor

tjzel commented Jun 19, 2024

Duplicate of #6066

@tjzel tjzel marked this as a duplicate of #6066 Jun 19, 2024
@tjzel tjzel closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@kirillzyusko
Copy link
Contributor Author

Have you tried the RC release to see if it works for you?

@tjzel No, not yet. But it was in my To Do list 🙃

Though it's worth to note, that I created a patch from your PR and it fixed a problem in my case (but I haven't tested a new RC yet).

We released it in an RC version because I'm not absolutely certain it works in all scenarios.

Well, even if it works not in all scenarios, then later on you can release a new version with additional fixes 😁

I understand, that you want to keep as small as possible releases and don't publish an official release for each new important bug fix, but this commit is actually very important, because it break a compatibility with all old versions (basic functionality works for sure, but I think many people were relying on capturing external objects in closures) 😔

@tjzel
Copy link
Contributor

tjzel commented Jun 19, 2024

No, that's actually not the case here. I'd normally release it in a patch immediately, but I had some reasons regarding the changes in our repo structure that made me postpone it.

However, when I went over them right now it turns out they are actually invalid. I will try to publish the patch later today 😃

@tjzel
Copy link
Contributor

tjzel commented Jun 19, 2024

3.12.1 containing the fix is out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing info The user didn't precise the problem enough Missing repro This issue need minimum repro scenario Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS
Projects
None yet
Development

No branches or pull requests

2 participants