buildo / react-components

Collection of general React components used in buildo projects.
http://react-components.buildo.io/
MIT License
157 stars 21 forks source link

[KitchenSink] loading time is high #549

Closed gabro closed 8 years ago

gabro commented 8 years ago

description

When loading a component page, it may take up to a few seconds to render the page. After speaking to @veej, this is because we load README files for each component directly from github.

specs

Since there's no real reason for loading content over the internet, we could just include the READMEs in the build.

/cc @FrancescoCioria @ascariandrea

FrancescoCioria commented 8 years ago

I totally agree with this pain point.

Since there's no real reason for loading content over the internet, we could just include the READMEs in the build.

First of all let's write here which requirements was I trying to solve (and successfully solved) when I implemented the showroom (remember: Drone CI didn't exist yet):

  1. the showroom should be easy to maintain:
    • the maintainer shouldn't manually update the showroom any time an example is updated
    • the maintainer shouldn't manually update the showroom any time a readme is updated
    • the maintainer shouldn't manually update the showroom any time a component is updated
      1. the showroom should include other buildo components from external repos (rc-datepicker, ...)

the maintainer shouldn't manually update the showroom any time an example or the readme is updated

Solved this by fetching the examples from the internet and

the maintainer shouldn't manually update the showroom any time a readme is updated

Solved by fetching the code from the internet and automatically generating the readme on-the-fly

the maintainer shouldn't manually update the showroom any time a component is updated

I didn't solve this

the showroom should include other buildo components from external repos (rc-datepicker, ...)

Automatically solved by above solutions.

After DroneCI arrived and after the last changes by @veej and @ascariandrea we are in this situation:

New requirements This how I would change the requirements today:

New specs draft

the showroom should be updated after any change on brc repo

already done by DroneCI

it's "ok" if showroom has to be manually updated after a change on an external component

yee, no specs for this :P

examples/code/readmes should not be fetched from internet

this is easy for brc components: bundle examples (already done), code (already done) and readmes (todo) in the build.

for external components we could:

@gabro @veej @ascariandrea do you agree with requirements? do you have any comments about the proposed specs?

veej commented 8 years ago

it's "ok" if showroom has to be manually updated after a change on an external component

IMO it would be great if we can find a way to have a new B-R-C build after a change in external components. Isn't it possible to have the CIs communicating in some way? (something like rc-datepicker CI calling the b-r-c CI after any successful build)

FrancescoCioria commented 8 years ago

good idea! I think we can do it by restarting the CI with a POST. @gabro do you think it's doable? can you help @veej on this?

ascariandrea commented 8 years ago

What about if "externals" were "submodules"? I know everyone scares when I suggest such a solution :(

gabro commented 8 years ago

What about if "externals" were "submodules"

what part of the problem are you solving with submodules? How would the proposed solution work?

gabro commented 8 years ago

@veej @FrancescoCioria The CI solution should be super easy, using the downstream plugin of drone

gabro commented 8 years ago

Only overhead is to add drone on every external component, but it should take a few minutes

ascariandrea commented 8 years ago

This is what scares me: "react-input-children": "buildo/react-input-children" in the package.jsonand that's why I suggest to use submodules. We're going out with this project and I would avoid this possible causes of bugs.

They will resolve the manual trigger of update the showroom when an external change (as the CI step would do) and we're going to have more control of what would happen managing the external version explicitly. Otherwise I know git-modules are broken, but an alternative exists: git subtree and maybe @gabro has more information to valuate it.

We could add the semver in the package.json, but I see the external more like "modules" than "dependencies".

gabro commented 8 years ago

We're going out with this project and I would avoid this possible causes of bugs.

what causes bugs? The fact that we depend on master? How would this change in case of a submodule?

They will resolve the manual trigger of update the showroom

well, it's not manual if the CI does it, isn't it?

git subtree

git subtree doesn't enforce a dependency, it just helps when merging code from an external repo. The "merge" step needs to be done manually anyway.


In general I think the problem of breaking stuff is real, but it's solved by having periodic releases of brc and not depending on master.

FrancescoCioria commented 8 years ago

This is what scares me: "react-input-children": "buildo/react-input-children" in the package.json

We should install these external components from github as dev-dependency (as we're already doing for rc-datepicker). I think we can add a "gh-react-input-children": "buildo/react-input-children" to devDependencies and avoid conflicts.

The other option would be to move the showroom code to another repo: I did the showroom on this repo because it was easier (we had circular dependencies between our libraries in that period). Now (thank to the CI) we could easily move the showroom to another repo and take every component (brc too) from master.

gabro commented 8 years ago

yes, triggering an external showroom repo from anywhere our components live is doable.

ascariandrea commented 8 years ago

well, it's not manual if the CI does it, isn't it?

Yep, @FrancescoCioria asked for feedbacks on actual workflow and this is my suggestion.

what causes bugs? The fact that we depend on master? How would this change in case of a submodule?

Depends directly on master could cause bugs and we experienced them. With submodule you can fix the commit, like you would do by "react-input-children": "buildo/react-input-children#$commit-hash" but there are a documented shared workflow that community knows, less error prone than manually updating an hash.

FrancescoCioria commented 8 years ago

In general I think the problem of breaking stuff is real, but it's solved by having periodic releases of brc and not depending on master.

just to be clear I'll repeat it again: brc is not depending on external libraries taken from GitHub. The showroom is.

What's the difference? the difference is: if you install b-r-c in your project you will not depend in any way to rc-datepicker (which is not used by any brc component) and you will depend on react-input-children and react-flexview (used by some components) but to precise npm versions (/lib not /src). You can check the "dependencies" object in the package.json to see every brc dependency