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

Animated.Value.interpolate results in NaN when output is in radians #36608

Closed
j-piasecki opened this issue Mar 23, 2023 · 5 comments
Closed

Animated.Value.interpolate results in NaN when output is in radians #36608

j-piasecki opened this issue Mar 23, 2023 · 5 comments
Assignees
Labels
API: Animated Resolution: Fixed A PR that fixes this issue has been merged.

Comments

@j-piasecki
Copy link
Collaborator

j-piasecki commented Mar 23, 2023

Description

When Animated.Value.interpolate is used with radians as the output units, like so:

interpolate({
  inputRange: [0, 2],
  outputRange: ['0rad', '6.28rad'],
});

it returns NaN instead of the correct values.

It happens on both, the old and the new architecture.

React Native Version

0.72.0-rc.0

Output of npx react-native info

System:
    OS: macOS 12.6
    CPU: (8) arm64 Apple M1 Pro
    Memory: 331.55 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
    Watchman: 2023.01.16.00 - /opt/homebrew/bin/watchman
  Managers:
    CocoaPods: 1.11.3 - /Users/jakubpiasecki/.rvm/gems/ruby-2.7.6/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1
    Android SDK:
      API Levels: 24, 26, 28, 29, 30, 31, 32, 33
      Build Tools: 26.0.3, 28.0.3, 29.0.2, 30.0.2, 30.0.3, 31.0.0, 32.0.0, 32.1.0, 33.0.0
      System Images: android-28 | Google ARM64-V8a Play ARM 64 v8a, android-32 | Google APIs ARM 64 v8a, android-33 | Google APIs ARM 64 v8a
      Android NDK: Not Found
  IDEs:
    Android Studio: 2022.1 AI-221.6008.13.2211.9514443
    Xcode: 14.2/14C18 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.14 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.2.0 => 18.2.0
    react-native: 0.71.0 => 0.71.0
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

Use the code below. It uses degrees to show how it should look, replace outputRange: ['0deg', '360deg'] with the version commented out to see it broken.

Snack, code example, screenshot, or link to a repository

import React, { Component } from 'react';
import { Animated } from 'react-native';

export default class Example extends Component {
  private rotation: Animated.Value;
  private rotationDegrees: Animated.AnimatedInterpolation<number>;

  constructor(props: Record<string, unknown>) {
    super(props);
    this.rotation = new Animated.Value(0);

    this.rotationDegrees = this.rotation.interpolate({
      inputRange: [0, 2],
      outputRange: ['0deg', '360deg'],
      // outputRange: ['0rad', '6.28rad'],
    });

    Animated.timing(this.rotation, {
      toValue: 2,
      duration: 2000,
      useNativeDriver: true,
    }).start();

    this.rotationDegrees.addListener(console.log);
  }

  render() {
    return (
      <Animated.View
        style={{
          transform: [{ rotate: this.rotationDegrees }],
          width: 200,
          height: 200,
          backgroundColor: 'red',
        }}
      />
    );
  }
}
@javache
Copy link
Member

javache commented Mar 23, 2023

Is this happening on both iOS and Android? Can you provide a failing test in https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Animated/__tests__/Interpolation-test.js that exposes this?

We have a test similar to this use-case which passes:

    const interpolation = createInterpolation({
      inputRange: [0, 1],
      outputRange: ['-100.5deg', '100deg'],
    });

Or is this happening at a later layer where we convert degrees into radians?

@javache javache self-assigned this Mar 23, 2023
@j-piasecki
Copy link
Collaborator Author

It's happening on both Android and iOS.
I tried to add the test, but the exact same config from the code above works in the tests.

I'm not sure at what point exactly it breaks, but it works when useNativeDriver is set to false, so it probably explains why the test would pass (also, native driver with degrees also works, it breaks when using radians).

javache added a commit to javache/react-native that referenced this issue Mar 26, 2023
Summary:
This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Differential Revision: D44406034

fbshipit-source-id: ea95f557d594a1f55bedd7c5e3a95ef36bc68ff2
javache added a commit to javache/react-native that referenced this issue Mar 27, 2023
Summary:
Pull Request resolved: facebook#36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: 23d90b7e25a502a705c7e74fd9b939f86b1ff93e
javache added a commit to javache/react-native that referenced this issue Mar 27, 2023
Summary:
Pull Request resolved: facebook#36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: c821d573d69b3a2111cd846a94d8a00b1b9227ed
javache added a commit to javache/react-native that referenced this issue Mar 27, 2023
Summary:
Pull Request resolved: facebook#36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: ea0eec9421b91d200024371696d4d2406a4f8767
facebook-github-bot pushed a commit that referenced this issue Mar 27, 2023
Summary:
Pull Request resolved: #36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue #36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: fe0f3df16f2b8ec6c31f9359e4706cacc72b9951
@blakef blakef added Resolution: Fixed A PR that fixes this issue has been merged. and removed Needs: Triage 🔍 labels Apr 4, 2023
@blakef
Copy link
Contributor

blakef commented Apr 4, 2023

Looks like it just missed inclusion in 0.72.0-RC0.

@redpanda-bit
Copy link

redpanda-bit commented May 3, 2023

Hey @j-piasecki it's working fine for me on both ios and android.

correction: I was able to reproduce on 0.72.0-rc.0. Why was the PR closed without merge?

cc: @javache

@javache
Copy link
Member

javache commented May 5, 2023

By default we only merge to master, if you think a fix should be merged in the RC, please comment in the release thread: reactwg/react-native-releases#54

@javache javache closed this as completed May 5, 2023
jeongshin pushed a commit to jeongshin/react-native that referenced this issue May 7, 2023
Summary:
Pull Request resolved: facebook#36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: fe0f3df16f2b8ec6c31f9359e4706cacc72b9951
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
Summary:
Pull Request resolved: facebook#36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue facebook#36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: fe0f3df16f2b8ec6c31f9359e4706cacc72b9951
fortmarek pushed a commit that referenced this issue Nov 8, 2023
Summary:
Pull Request resolved: #36645

This broke while changing the AnimatedInterpolation back in D40571873 and D40632443, as I assumed the native side would be able to correctly handle values such as '1rad'. However these were being sent over as strings, and were thus using the string interpolation path, which does not work here.

Instead, handle both `deg` and `rad` explicitly when generating the config in JS.

Resolves issue #36608

Changelog: [General][Fixed] Resolves Animated.Value.interpolate results in NaN when output is in radians

Reviewed By: yungsters

Differential Revision: D44406034

fbshipit-source-id: fe0f3df16f2b8ec6c31f9359e4706cacc72b9951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Animated Resolution: Fixed A PR that fixes this issue has been merged.
Projects
None yet
Development

No branches or pull requests

5 participants