requestAnimationFrame returns Unix timestamp instead of elapsed time

[jamesseanwright]

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

OS: Linux 4.4
Node: 6.9.4
Yarn: Not Found
npm: 3.10.10
Watchman: Not Found

Packages: (wanted => installed)
react-native: 0.48.4 => 0.48.4
react: 16.0.0-alpha.12 => 16.0.0-alpha.12

Target Platform: Android 7 (SDK version 25)

Steps to Reproduce

Create a simple React component that invokes requestAnimationFrame once, stores the time parameter in state, and renders the result:

import React from 'react';

import {
    AppRegistry,
    StyleSheet,
    View,
    Text
} from 'react-native';


const styles = StyleSheet.create({
    container: {
        flex: 1,
        justifyContent: 'center',
    },
})

class App extends React.Component {
    constructor() {
        super();

        this.state = {
            time: 0,
        };

        requestAnimationFrame(time => (
            this.setState({
                time,
            })
        ));
    }

    render() {
        const { showProgressBar } = this.state;

        return (
            <View style={styles.container}>
                <Text>{this.state.time}</Text>
            </View>
        );
    }
}

AppRegistry.registerComponent('App', () => App);

Expected Behavior

The time parameter should be the time elapsed since the "page" "loaded" (navigated to current view or activity?) as is the case in Chrome and Firefox (this value should be performance.now(), according to MDN.

Actual Behavior

The time parameter resolves to the current Unix timestamp (i.e. Date.now()).

Reproducible Demo

https://github.com/jamesseanwright/react-native-requestanimationframe-bug

I'd be happy to submit a PR to fix this, but I'm unsure of how this project versions breaking changes. I can work around this issue, but it would be great to achieve parity with the browser API so that I can avoid platform-specific hacks.

Let me know your thoughts.

Cheers,
James

[janicduplessis]

Hi James, thanks for the detailled issue. Feel free to open a PR to fix this. For handling breaking change you can just add "BREAKING - " to your PR title so it will be clear to the person that does release notes. Parity with web APIs is important so the breaking change is fine here.

Let me know if you need help with anything.

[jamesseanwright]

@janicduplessis Sounds great, I'll get started! Could you assign this to me or mark it somehow to ensure that no one else opens a duplicate PR?

[jamesseanwright]

Hi all,

Unfortunately, I have not had a chance to look at this due to other commitments, and with my travels approaching in January, I won't be back into the development scene for a few months. Therefore, anyone who might want to tackle this can go for it, something of which I'd be greatly appreciative. Otherwise, I'll take another look when I'm back.

Cheers,
James

[brandontruggles]

Hello, I'd like to work on this!

Edit: Since I am new to contributing to this repo, could anybody please point me to the correct files/methods I should be looking at to facilitate this change?

Thank you!

[brandontruggles]

UPDATE: I've been looking at the following line in particular:

react-native/Libraries/Core/Timers/JSTimers.js

Line 156 in 3fa6482

callback(performanceNow());

It seems to me that the reason the callback parameter is returning the Date.now() value, is because the fbjs performanceNow library uses Date.now() as a fallback when performance.now() is undefined:

https://github.com/facebook/fbjs/blob/c69904a511b900266935168223063dd8772dfc40/packages/fbjs/src/performance/performanceNow.js

Would implementing this change require a rewrite to that library, or will Date.now() have to suffice as long as we perform the proper calculation (relative to view load)? Also, where would we place the logic to create a timestamp when a view is first loaded for calculating the elapsed time?

Thanks!

[santhosh77h]

@janicduplessis @sophiebits hi i was looking into this issue and found out from this line from debugger that window.document is undefined.

[TakaGoto]

Hello, Is this still being worked on can I take a stab at this?

[patrickkempff]

@TakaGoto go ahead. Feel free to open a PR to fix this :)

[Esemesek]

I've send fix to fbjs, but I wonder if it is better to fix it by adding performance polyfill in https://github.com/facebook/react-native/tree/master/Libraries/polyfills