-
Notifications
You must be signed in to change notification settings - Fork 1
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
Move connection hook to improve performance #143
Conversation
This might not be the best way to handle this, in particular the local PV is now limited but it does work for the majority of cases I am interested in testing at the moment
This reverts commit e25a569.
This PR currently prevents us from calculating rules at runtime, but we can't do this effectively anyway for many props (such as position, font etc). I think this should be looked at as a separate concern. |
Required mocking out multiple modules which can be reasonably painful. Some improvements could perhaps be made to the implementation
containerProps: any & { id: string }; | ||
widgetProps: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to stop these allowing any
through in the future
Interesting that this branch improves performance, as it seems a fairly minor change. As the performance is comparable to what you called 'hooked-components' I think this one is more useful. |
The connection hook is required at runtime, but many of the other hooks (useMacros etc) are not. Therefore, this PR moves the connection hook into a separate widget which is rendered after the load-time hooks in order to separate concerns. This appears to have provided an approximately 36% increase in speed over what is currently on the master branch when measuring both with production build and the profile flag (
npm run build -- --profile
)It also does not disrupt our current widget layout but does give us a method for concerning load-time and runtime concerns.