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

Add documentation in .md format #460

Merged
merged 82 commits into from
Nov 11, 2019
Merged

Add documentation in .md format #460

merged 82 commits into from
Nov 11, 2019

Conversation

jakub-gonet
Copy link
Member

@jakub-gonet jakub-gonet commented Nov 9, 2019

Add documentation in .md 🎉

  • Updated docs
  • Updated component-docs config
  • Minor "motivation" page rewrite
  • Removed meme 😢

Live docs can be viewed here

Jakub Adamczyk and others added 30 commits May 13, 2019 13:50
Motivation
In React Native Gesture Handler repo we have integration with travisCI to test if our Example apps are building and some detox e2e tests on iOS. I thought that it is not much work to add CI here as we already have config and project config is similar so it would most likely work out of the box with little tweaks.

Changes
Added YAML config file and Travis CI integration
I was recently using reanimated in react-navigation where I needed to mock it to be able to run it in Jest. It'll be great if the mock was in the library so we don't have to duplicate it in every project.

I looked through the typescript definitions and added a mock function for almost everything. I didn't use anything specific to a test runner such as Jest, so it should be possible to use the mock with any test runner.
Prevents crashing when referenced in libs like react-navigation.
glob-to-regexp does not escape backslashes as of version 0.4 which causes issues on windows machines. replacing backslashes with slashes only on windows fixes the issue.
concat#1 can be useful to convert a numerical animated value to a string.
I had a need for Math.log in an Animation calculation. This code all seemed to work and I think I found all the touch points, but I'm not well versed on the native side of things so please let me know if anything needs to change.
Since `__addChild` etc. are called so many times, using a for loop makes it faster by around 10ms at least (on iOS simulator, on device it might be even more).
This PR is a proof of concept on how to address issues like #160 #309.

As outlined in my comment, the problem with the current batched updates are that reanimated lags behind event processing by at least one event, in some cases up to 3-4. If using something like react-native-gesture-handler this is generally not noticeable as reanimated will be responsible for positioning all elements, but when deriving an animated value from a scroll event this would cause the elements positioned by reanimated to be out of sync with the scroll view.

To address the lag, this PR introduces a concept of "direct events", that will bypass the queue and apply its effects immediately. Due to my unfamiliarity with the codebase I didn't really know how to do this in a nice way without also doing a quite big refactor. This is a low invasive change to illustrate how it can be addressed, I'm happy to receive feedback on how this can be done in a cleaner way.

Test plan
Using @lindesvard's reproduction of the issue https://github.com/lindesvard/reanimated-scroll-perf on a physical device, applying these changes should remove the lag and stuttering.

Adverse effects
Since the direct effects will bypass the queue, it will not retain the same order. Given the declarative nature of reanimated I think this should be OK in most cases, but still worth mentioning. A possible workaround is to flush the queue before applying direct events.
The current way to determine direct events are by event name which is not particularly specific. In the odd case of naming collision, I don't see it having profound negative effects however.
This fixes typescript error `Cannot find name 'TransformStyle'.`.
This allow using the constant directly with `TurboReactPackage`.
Motivation
When creating multiple dynamic animations with react-native-reanimated the number of nodes that needs to be transported over the bridge is a performance challenge. One of the limitations of the current primitives offered by reanimated is the lack of function nodes. Being able to define node trees that can be reused will offer better performance. A previous attempt has been done in this pull request: #42

Implementation
The suggested implementation tries to limit changes to the current source code by adding new nodes without changing the existing implementation too much. Three new nodes have been added:

ParamNode
CallFunctionNode
FunctionNode
A few changes to the NodeManager to be able to force reevaluation when calling functions have been added.

The implementation uses a simple proc function in Javascript to create function nodes. Following is an example of how a function is defined and used.

// Defining the function
const mulXY = proc((x, y) => multipy(x, y));

// Using the function
const myX = new Animated.Value(12);
const myY = new Animated.Value(14);
const myResult = mulXY(myX, myY);
Technical
The solution I have used in this implementation is to implement the ParamNode as a node that holds a reference to other nodes - returning the referenced node's value when evaluated and setting the referenced node's value when changed.

Calling the function node is done using the CallFuncNode that is created when calling the function (on the JS side). When the CallFuncNode is evaluated, it performs the following steps:

Start a new context
Copy current argument nodes to the param nodes (using a Stack in the param node)
Evaluate the function node's expression
Pop the current argument nodes from the param nodes
End context
I have also changed NodeManager's invalidate strategy so that it will force a reevaluation of nodes when we are in a function call to avoid returning the same value for multiple functions calls with different parameter references.

Using a stack in the ParamNode ensures that we can create recursive functions.

Memory
A single proc node should be created as a global constant in javascript since they are immediately attached and will never be removed.

Platforms
Both Android and iOS platform-specific code has been added in addition to the required JS code.

Further plans
Rewriting functionality in reanimated called often should be considered. Moving complex spring functions to functions could also be considered but is not necessarily part of this PR.

Testing
Convert built-in derived functionality to use proc nodes and verify that all examples work nicely. I have already done this for interpolate and easings and it seems to be working fine. Verify that no memory leaks exist.

I believe that the current implementation supports caching of function evaluation - would be happy if someone else could confirm this.

Co-authored-by: osdnk <micosa97@gmail.com>
Ran into the issue here:
satya164/react-native-tab-view#845

This is because `ScrollView` doesn't have `.getNode()` but `Animated.ScrollView` does. 

This change fixes the error. I think this is the correct fix.

cc @simonbuerger
replaced backticks(?) to "`"
Refactors the create animated component removing the deprecated componentWillMount lifecycle and moving the attachProps to the constructor.
wcandillon and others added 26 commits October 31, 2019 09:33
fixes #343

This can enable us to save some unnecessary node creation but more importantly, it will enable us to benefit from the Exhaustive Deps eslint rule. This rule expect a callback as first parameter: https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L64
This rule is incredibly useful to avoid bugs when using hooks that have dependencies.
Following #355 (comment) it's true that there is no easily accessible documentation about how to set up Jest tests with reanimated.
This change fixes the problem where a parent node were marked as dirty while adding connection between nodes instead of the children node. We should be only marking children node as if we mark parent node there is a chance that this would trigger updates in leave nodes (final nodes) that should not be affected as the children added to parent node cannot impact its value.

On top of that the marking change relealed a bug where we initialized loop ID inproperly. We should be initializing it with a value that'd allow the node to be evaluated at least once. It's been working before as every node would be dirty marked. With the above change the root nodes were no longer being marked but we'd still require them to be evaluated. The invalid initial setting for last loop ID was making us skip evaluation of those parent nodes.

In order to test this change I created the following sample app: https://gist.github.com/7fce6b4a891f0284baa17d0d63455495
In this app there is a button that creates a new view that hooks into an existing animated value. Before this change when the view was added we'd reevaluate all the nodes that depend on that value and hence we'd get a consol.warn being shown twice (first on the initial render and then second time when we add the view). WIth this change in the `call` node should not be evaluated more than once and only on the initial render.
* Fix ESLint setup and devDependencies

- Fix version of types/react-native
- Update husky and lint-staged

* Fix lint errors
We don't need static assets right now
@jakub-gonet jakub-gonet changed the title @kuba/docs updated Add documentation in .md format Nov 9, 2019
@osdnk osdnk merged commit 0197833 into software-mansion:docs Nov 11, 2019
@osdnk
Copy link
Contributor

osdnk commented Nov 11, 2019

love description of this commit xd

@jakub-gonet jakub-gonet deleted the @kuba/docs-updated branch November 11, 2019 17:12
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.

None yet