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

Performance: Improve Context system + style rendering for primitive and core components #60

Merged
merged 7 commits into from
Sep 25, 2020

Conversation

ItsJonQ
Copy link
Owner

@ItsJonQ ItsJonQ commented Sep 25, 2020

This update adds a new hook-based connect mechanic to the Context system.

Addresses some of the concerns originally surfaced here:
#57

Reducing Layers

This strategy was originally mentioned during the early phases of planning the style system:
#2 (comment)

However, a we opted for a pure HOC based strategy, with the understanding that a couple of extra layers is worth of automating prop handling.

I think connect may be valuable at the user/consumer level. However, we probably want to optimize as much as possible at the library level.

This update reduces the amount of layers from a fully connected context aware component (e.g. Flex) and the underlying div element that React outputs. Typically, this usually 2-3 layers:

`Flex` -> `View` -> `div`

For key instead of ...spreading

Other (potential?) performance enhancements were made at the prop consolidation step within the new useContextSystem hook. Notably, skipping JS object spreading in favour of a more manual for loop.

We may see some gains from this, as noted by @diegohaz in this Tweet:
https://twitter.com/diegohaz/status/1247462435516276742

Improving Flex

With the addition of HStack and VStack, the core Flex component has seem a massive uptick in usage. As such, we should optimize these core components as much as we can. In this update, I've opted to use CSS variables for child alignment, rather than using an internal Flex context.

Improving View

A tiny but interesting + important update. View is not longer connected to the Context system. I can't see an scenario where View may benefit from a system-wide context update 🤔 . (We may revisit this in the future if that happens).
By disconnecting it from the Context system, View avoids potential unnecessary re-rendering based on context/hook updates.

Renaming ComponentsProvider to ContextSystemProvider

The Context system is much more refined now. It's being used more throughout the G2 system.
Therefore, I think the name ContextSystemProvider feels more intuitive.

@vercel
Copy link

vercel bot commented Sep 25, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/itsjonq/g2/eze9uwzyb
✅ Preview: https://g2-git-try-perf-improvements-for-primitives.itsjonq.vercel.app

@ItsJonQ ItsJonQ merged commit d7e09fe into master Sep 25, 2020
@ItsJonQ ItsJonQ deleted the try/perf-improvements-for-primitives branch December 10, 2020 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant