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 Audit #744

Closed
jossmac opened this issue Feb 27, 2019 · 10 comments
Closed

Performance Audit #744

jossmac opened this issue Feb 27, 2019 · 10 comments
Assignees

Comments

@jossmac
Copy link
Member

jossmac commented Feb 27, 2019

Our lighthouse score is pretty bad...

Lighthouse Performance Insights

@emmatown
Copy link
Member

While I completely agree we need to improve load performance, I don’t think doing SSR is worth it for the Admin UI. There are a bunch of things that rely on layout and local storage and things and while we could solve all the problems, I’m not sure the time and complexity cost would be worth the likely minimal to no performance benefit or potentially worse performance.

@jesstelford
Copy link
Contributor

jesstelford commented Feb 28, 2019

I'm working on parts of this this now 👍

@jesstelford jesstelford self-assigned this Feb 28, 2019
@jossmac jossmac changed the title SSR Support / Perf Performance Audit Feb 28, 2019
@jossmac
Copy link
Member Author

jossmac commented Feb 28, 2019

@mitchellhamilton fair enough. Title updated.

@VinayaSathyanarayana
Copy link
Contributor

Need to look at "Enormous Network Payload" of 13MB first

@jesstelford
Copy link
Contributor

There is some useful lazy-loading code in https://github.com/keystonejs/keystone-5/pull/745 which we can cherry-pick onto master for some perf wins (experiments show it removes ~3MB from first load).

@emmatown
Copy link
Member

emmatown commented Mar 1, 2019

FYI, I'm working on this, gonna take a slightly different approach to #745 with suspense. I'll probably want to cherry-pick the accessing field types from adminMeta stuff from #745 though.

@jesstelford
Copy link
Contributor

I'm also working on building the AdminUI with next.js to get all the perf benefits it can convey with static building, etc.

@jesstelford
Copy link
Contributor

jesstelford commented Mar 10, 2019

I've been playing around with switching to next.js, and results so far are promising!

Current master

Screen Shot 2019-03-11 at 10 58 59 am

With Next 8

Screen Shot 2019-03-11 at 11 05 19 am

@jesstelford
Copy link
Contributor

I don’t think doing SSR is worth it for the Admin UI.

Next.js has SSR built in - it takes zero extra effort to get started.

There are a bunch of things that rely on layout and local storage and things and while we could solve all the problems, I’m not sure the time and complexity cost would be worth [it].

I spent some time this weekend ironing this out - there were very few changes needed 👍

likely minimal to no performance benefit or potentially worse performance.

See the above screenshot of the perf benefits 👆

@JedWatson
Copy link
Member

JedWatson commented Mar 12, 2019

I'm doing some repo cleanup and closing issues that aren't specifically actionable.

Since this is more of a broad concern, I'm going to close it. Let's continue to focus on performance and open actionable issues we (and contributors) can work on.

In the meantime I'm expecting #749 will make a pretty big difference when it lands.

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

No branches or pull requests

5 participants