elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.63k stars 8.22k forks source link

Move kibana-ui-framework into kibana repo. #8867

Closed cjcenizal closed 7 years ago

cjcenizal commented 8 years ago

Benefits

After discussion with @spalger, @w33ble, and @epixa, we made the following decisions:

Deploy docs to GitHub page

In an ideal world, we can deploy all development-related resources to our GitHub page (elastic.github.io/kibana), e.g. UI Framework docs, API docs, plugin development docs, JSDocs.

For the time being, this can just consist of the UI Framework docs.

If we change our minds and decide we need to deploy to the Elastic domain, we should talk to the infra team.

devDependency licenses are a non-issue

The UI Framework has some devDependencies (e.g. node-sass) which have dependencies that don't have supported licenses (e.g. "Unlicense"). This won't affect us, since these don't make it into our distributed builds.

The Kibana UI is for Kibana only

In other words, it won't be used in any other product. It won't be used in other products because they're so distinct that any benefits gained from shared CSS would be negligible, and possibly restrict development.

Kibana UI code goes into src/ui-framework

The code that consumes it lives in src, so it makes sense to group them together.

Outstanding questions

1 package.json or 2?

The UI Framework encapsulates documentation which is built using dependencies that are not used by the Kibana web app. Should Kibana UI have its own package.json to define these dependencies? Or should we group all dependencies together in the root package.json?

ycombinator commented 8 years ago

Re: the package.json question, I'm +1 for the root package.json option. That way there's one place for devs to look for all dependencies rather than setting a precedent where some sub-folders can have their own package.jsons.

simianhacker commented 8 years ago

Once you're part of Kibana you're part of the package.json +1

cjcenizal commented 8 years ago

@ycombinator Can we take that a step further an enumerate the negative effects this would have? Let's lay out the benefits and liabilities each option has and base our decision on that.

ycombinator commented 8 years ago

@cjcenizal Sure, I imagine the negative is that there's too many dependencies in the root package.json without clearly knowing which dependencies are required by which part of the codebase? I'm not really sure if that's a big deal though but others may have thought of other negatives?

kimchy commented 8 years ago

++ on moving the code to Kibana repo, great stuff. All docs go on elastic.co, no open question here :).

cjcenizal commented 8 years ago

@ycombinator I'm also wondering what are the negative effects of setting a precedent where some sub-folders have their own package.json? We could establish criteria for that, e.g. only sub-folders which contain source code which requires its own build process can have a package.json.

ycombinator commented 8 years ago

Only that there are now multiple places to look for dependencies. Probably just a personal choice that I prefer to look in a single place, though. So even if we went with separate package.jsons in sub-folders, I'll survive 😄

ycombinator commented 8 years ago

In case it wasn't clear from my earlier comment, I don't have strong opinions here. In hindsight I probably shouldn't have commented at all 😄. I'd be absolutely fine going with whatever approach is chosen.

tylersmalley commented 8 years ago

In the past, the team has chosen to create separate repositories to maintain isolation. I am in favor of moving this into the Kibana repository, but we need to figure out how to maintain isolation and not end up with hundreds of direct dependencies. One example I have seen of this being handled is using Lerna. It's a simple concept, and really the package helps with publishing and linking libraries. One issue I do see is because we have a top-level package.json with all our Kibana dependencies, we would need to make sure there is not any leak to the /packages projects.

@spalger previously mentioned he has not had much luck with this approach in the past. Maybe he can clarify.

epixa commented 8 years ago

The solution to the problem of "hundreds of direct dependencies" is to have fewer dependencies. Every single dependency we add to the project is baggage - sure, it's a single line to add a new dependency to a package.json, but that single line is simply obfuscating the reality that Kibana is now however-many-lines-were-in-that-dependency more complicated than it was before.

Making it easier to add dependencies or have different versions of the same dependencies in the project isn't a feature, it's a problem.

Beyond being philosophically contrary to how we want to build Kibana, multiple package.jsons introduce other issues:

Installing or updating Kibana dependencies is now multiple commands, which is just downright annoying.

Different parts of our code could be relying on different versions of the same dependency, which increases the cognitive overhead of working on different parts of the codebase, and it increases the payload size of our bundles.

We could use or build abstractions on top of npm to handle things like installing/updating dependencies across the project, keeping dependency versions in check when they are used in multiple places, or flattening the overall module tree based on the sum of all of the dependency trees across the various directories, but what justifies that added complexity?

Where is the benefit?

cjcenizal commented 8 years ago

@epixa, thanks for the good points. We discussed this as a group and worked out some solutions to the challenges of moving to a single package.json.

How to use a single package.json

Context

When we move the UI Framework inside of the Kibana repo, the repo will now be responsible for two separate bundles (Kibana and the UI Framework docs site).

~23 new devDependencies

We’ll need to add about 23 new devDependencies to our package.json. These dependencies are mostly related to React and Redux, which are used by the UI Framework docs site.

The main question this raises is: “If we use these dependencies in Kibana too (e.g. React), how will our workflow be affected?”

Impact of shared dependencies

Shared dependencies carry the following implications:

How can we design our process to make it easy for a dev to upgrade or remove a dependency and remember to make all of the necessary changes?

Managing shared dependencies

If Kibana incorporates React, then the UI Framework will become responsible for publishing React UI components. In this case, they’ll need the same version of React and related dependencies, so a shared package.json will be helpful.

For other shared dependencies, we’ll need to run certain tasks in tandem to make developers aware of when either bundle has been broken by an upgraded or removed dependency (@pickypg please add any missing details here):

Moving docs to elastic.co

As a company, we want all docs to be on elastic.co (both for users and contributors). This would, for example, apply to the UI Framework docs and future documentation for the Kibana Plugin API.

I think we can do this with the following steps:

tbragin commented 8 years ago

As a company, we want all docs to be on elastic.co (both for users and contributors). This would, for example, apply to the UI Framework docs and future documentation for the Kibana Plugin API.

@cjcenizal @epixa @debadair I think it's worthwhile noting that we don't yet have a natural place to put Kibana developer-facing docs in the current Kibana guide. We have a plugin users section (installing/updating/removing plugins): https://www.elastic.co/guide/en/kibana/current/kibana-plugins.html

So this feels like either a net-new section in existing Kibana docs (the way Logstash docs do it) or a whole new "book" for plugin developers (the way Elasticsearch does it).

epixa commented 8 years ago

I think a separate set of docs is the way to go here, like the ES docs.

cjcenizal commented 7 years ago

Addressed by https://github.com/elastic/kibana/pull/9192