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

fix: ssr support (#1890) #1891

Merged
merged 2 commits into from
Apr 6, 2021
Merged

Conversation

nandorojo
Copy link
Contributor

Description

Fix #1890

Changes

Don't call window in the constructor of JS Reanimated, since this isn't available on the server.

Test code and steps to reproduce

One-line change, which reverts back to what was already there in 2.0.0-rc.0. Let me know if this is needed, but given that it's simply a regression patch, I figure it isn't.

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Added TS types tests
  • Added unit / integration tests
  • Updated documentation
  • Ensured that CI passes

Comment on lines 16 to 19
this.timeProvider = {
now: () =>
process.env.JEST_WORKER_ID ? Date.now() : window.performance.now(),
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeProvider.now() is called in every frame on the web implementation, so could you change this to a little bit faster version without if condition? Something like this:

this.timeProvider = {};
if(process.env.JEST_WORKER_ID) {
  this.timeProvider.now = () => Date.now();
}
else {
  this.timeProvider.now = () => window.performance.now();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes more sense, I went with a ternary to stay consistent. Thanks for taking this on and making the fix!

@piaskowyk piaskowyk merged commit 5a449ec into software-mansion:master Apr 6, 2021
@jakub-gonet jakub-gonet mentioned this pull request Apr 6, 2021
piaskowyk pushed a commit that referenced this pull request Apr 6, 2021
Fixes prettier issue introduced in #1891.

¯\\_(ツ)_/¯
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2.0.0] Server-side rendering breaks
2 participants