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

Optimizing page rendering via hiding or deactivating CodeMirror until interaction #86

Closed
mik01aj opened this issue Feb 16, 2016 · 27 comments

Comments

@mik01aj
Copy link
Collaborator

mik01aj commented Feb 16, 2016

Oringinal title for this issue was: Rendering the styleguide in Chrome is awfully slow (because of CodeMirror)

That's a screenshot of the timeline initial render:

image

Note that the first render is shown only after 45 seconds. The styleguide has around 40 components.

I looked at the timelines, and from what I understand, CodeMirror somewhat updates the DOM in a way that might change the width of the toplevel container. Or maybe just does some pretty ineffective DOM operations (like mixed read/writes).

Interestingly Firefox doesn't have this problem and shows the whole page after around 8 seconds.

@sapegin
Copy link
Member

sapegin commented Feb 16, 2016

Maybe autogrowing height of CodeMirror is the reason?

@robhrt7
Copy link

robhrt7 commented Feb 16, 2016

What do you expect, putting 40 components on one page.

@sapegin
Copy link
Member

sapegin commented Feb 16, 2016

We can make editors collapsed by default.

@robhrt7
Copy link

robhrt7 commented Feb 16, 2016

It's also possible to activate the editor on click, so the source code will be shown always.

But really, I would consider allowing to create multiple pages. One page style guide solution does not scale in many ways.

@sapegin
Copy link
Member

sapegin commented Feb 16, 2016

Then we’ll have to highlight it and pray that it want jump too much on enabling ;-)

@mik01aj
Copy link
Collaborator Author

mik01aj commented Feb 17, 2016

@operatino, it can be fast. Look at https://react-bootstrap.github.io/components.html for example. They have 110 examples on the page (btw, they don't show CodeMirror initially, maybe for the same reason 😉). I do agree that it would be nice to be able to select some component and view it separately, but that's an independent feature request. It should be fast even when they are all displayed on one page.

@sapegin, I think enable-on-click could be a good idea. But maybe there's a simpler solution (e.g. fixing some widths in CSS)? It's strange that Firefox renders the page so quickly.

@robhrt7
Copy link

robhrt7 commented Feb 17, 2016

@mik01aj you're comparing plain CSS bootstrap vs complex single page app with lot's of JS base components on it.

Fixing "some widths in CSS" is just a temp hack. This also limits any client side code features, since if you're already at full capacity, there's no place for adding some more advanced stuff.

@sapegin
Copy link
Member

sapegin commented Feb 17, 2016

It’s fast because they do not show editors by default.

But there are three things we can do:

  1. Make editors collapsible, good to have anyway.
  2. Improve performance, alway good to do.
  3. Activate editors on click, only if there’s an easy way to do it.

@operatino They do exactly the same thing: show bunch of React components on a single page, they just hide editors.

@mik01aj
Copy link
Collaborator Author

mik01aj commented Feb 17, 2016

@operatino that's react bootstrap, rendered via JS (the doc page is a huge react component) and all the code examples are editable. It's pretty similar.

The problem here is not JS code, JS is fast (15.4% of the run time), it's the DOM and layout calculations that are so slow (83.5%).

@mik01aj mik01aj changed the title Rendering the styleguide in Chrome is awfully slow Rendering the styleguide in Chrome is awfully slow (because of CodeMirror) Feb 17, 2016
@robhrt7
Copy link

robhrt7 commented Feb 17, 2016

Okay, I've missed that you shown React Bootstrap, but look deeper. Every component there is server side rendered, which boosts the performance a lot.

Also, it's still Bootstrap, which is a set of simple components. In a decent project, you get a lot more complex components with more functionality, which again, doesn't scale at all.

@mik01aj
Copy link
Collaborator Author

mik01aj commented Feb 17, 2016

@operatino would you mind opening a new issue for separate-pages-per-component feature? I'd like to focus on performance in this ticket.

@sapegin
Copy link
Member

sapegin commented Feb 17, 2016

It’s much bigger feature: grouping components + table of contents + something else :-)

@robhrt7
Copy link

robhrt7 commented Feb 17, 2016

I even have a solution already - https://github.com/sourcejs/sourcejs-react-bundle-example ;)

Which is a higher level wrapper on top of styleguidist. Treat it as an idea for scaling, not the promotion of some specific implementation.

@robhrt7
Copy link

robhrt7 commented Feb 17, 2016

In first React implementation https://github.com/szarouski/sourcejs-react (quite limited) we also had server side rendering, which I would really like to pair up with styleguidist somewhere in future.

@sapegin
Copy link
Member

sapegin commented Feb 17, 2016

@operatino That would be cool!

@mik01aj
Copy link
Collaborator Author

mik01aj commented Mar 4, 2016

The separate-pages-per-component feature suggested above by @operatino is reported in #98.

@sapegin do we have any decision on this one? If we decide that we should hide CodeMirror initially, i'd rename this issue and close #12 in favour of this one.

@sapegin
Copy link
Member

sapegin commented Mar 4, 2016

Yes, with an option to change default visibility. But I’m not sure about the default value.

@mik01aj mik01aj mentioned this issue Mar 4, 2016
6 tasks
@mik01aj mik01aj changed the title Rendering the styleguide in Chrome is awfully slow (because of CodeMirror) Optimizing page rendering via hiding or deactivating CodeMirror until interaction Mar 4, 2016
@MoOx
Copy link
Contributor

MoOx commented May 2, 2016

I have to admit that styleguidist just currently offer a pretty bad UX just because of this speed issue.
I have "just" 20 components and initial page render takes seconds to load.
Compared to that my real app with 200 instances of components render in 1 or 2 seconds.

@sapegin
Copy link
Member

sapegin commented May 2, 2016

@MoOx That’s why we have a “help wanted” label here.

@mik01aj
Copy link
Collaborator Author

mik01aj commented May 4, 2016

To summarize, we'd like to have

  • a new config option, e.g. exampleCodeShown
  • that option should be true by default (we can discuss the default, but I think if we want to make it a semver-minor version we should mimic the current behaviour and just focus on performance. We could change the default in the next major version if we want to.)
  • have the Editor component just syntax-highlight the code in a <pre> when exampleCodeShown: true
  • have the Editor component show a "show code" button when exampleCodeShown: false
  • have the Editor component become a CodeMirror component on first click
  • [maybe] we'd like to have also a "hide code" button then too

@MoOx
Copy link
Contributor

MoOx commented May 8, 2016

Is there any quick workaround to remove codemirror (even if that's involved removing the entire code below demo parts). Currently this project is more and more unusable for me, just with 20 components...

@mik01aj
Copy link
Collaborator Author

mik01aj commented May 9, 2016

You can override the Editor component with and empty div, see the FAQ in Readme.

@MoOx
Copy link
Contributor

MoOx commented May 16, 2016

I successfully used a "pre" tag for the editor, but perfs are still very bad.

@mik01aj
Copy link
Collaborator Author

mik01aj commented May 17, 2016

That's a surprise to me! Could you send us a screenshot of the timeline?

@mik01aj
Copy link
Collaborator Author

mik01aj commented Jun 8, 2016

I tried recording the timeline again, and have seen a nice surprise!

image

I have 63 components now and the page loads in around 8 seconds. But around 60% of this time is CodeMirror initialization (the highlighted frame triggers CodeMirror.fromTextArea for all code examples), so there's still much room for improvement!

@tizmagik
Copy link
Collaborator

tizmagik commented Jul 7, 2016

Shall we close this, given #150 ?

@mik01aj
Copy link
Collaborator Author

mik01aj commented Jul 7, 2016

Yes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants