search

firefox-devtools / rfcs

RFCs for changes to DevTools
15 stars 17 forks source link

New devtools-components bundle #31

Closed nchevobbe closed 6 years ago

nchevobbe commented 6 years ago

In Bug 1406415, we want to experiment having a bundle of the devtools-components package from devtools-core.

The motives behind that for now is to be able to use the Tree component we have there. More specifically, @gabrielluong would like to use it for the markup view (the other Tree components in mozilla-central are not a good fit for this purpose).

In the future, we want to move the StackTrace and the Frame components to devtools-components so we can use them in devtools-reps - in the Error Rep - to have proper sourcemapped error. This would also allow us to get contributions from library authors/users in order to have icons and dedicated logic (e.g. collapsing frames), like the Debugger successfully did. Those would then be republished to mozilla-central as a bundle, and will be used anywhere we have a Stacktrace: Console, Netmonitor, and even in the Debugger Callstack panel as well. The original components in m-c would be deleted.

This raises some concerns because the Tree component is already imported in devtools-reps AND debugger. Doing another bundle for devtools-components would mean we have the source code for this component 3 times.

I guess this is something that would be solved by our Github/m-c multi-mono repo architecture, but this is a bit early to assert.

What are people opinions on this ?

jasonLaster commented 6 years ago

I would be happy to start excluding the devtools-component from the debugger bundle and using the M-C version.

juliandescottes commented 6 years ago

I would be happy to start excluding the devtools-component from the debugger bundle and using the M-C version.

Then the dependency management gets more complicated. This means that the debugger always needs to be compatible with the version of devtools-component available in mozilla-central.

But it is true that this is about finding a good balance between duplication and dependency issues.

Our options:

I was the one who criticised the "duplication" approach initially, but it might be the easiest for us to handle for now. The drawbacks of this approach are that we have 1k LOC duplicated, and we need to release one more bundle from GitHub.

The fact that we want to put more things into this devtools-components makes the duplication issue a concern for the future, so hopefully this will be solved by a new approach with m-c -> GitHub synchronization.

I think overall I am concerned with moving more development to GitHub while we struggle right now with the GitHub/mozilla-central integration. Is moving back the tree component to m-c an option here?

ochameau commented 6 years ago

Is moving back the tree component to m-c an option here?

+1

nchevobbe commented 6 years ago

Is moving back the tree component to m-c an option here?

It could be, but then I'm unsure how the ObjectInspector and the Debugger (source tree), would consume it (especially in launchpad mode).

It's not only about the Tree component, Stacktrace and Frame were candidates to be moved to devtools-components so we can use them in the Error rep.

I wanted Stacktrace and Frame in Github so we can benefit from the library work that happened in the debugger (icons, frame collapsings, …) and make the debugger community contribute more to devtools-core.

If we can achieve theses goals without code duplication, and save us from doing bundle releases, I'm all in. But for now, I have hard time to figure out how to do that without having those components right into github.

ochameau commented 6 years ago

What would you be missing if devtools-components was in m-c ? I only see that the contribution workflow (bug+mozreview vs GH) is different. Is there more? Do we also have a build system issue? If yes, how do we manage to not have build system issue for react components we have in tree?

juliandescottes commented 6 years ago

It could be, but then I'm unsure how the ObjectInspector and the Debugger (source tree), would consume it (especially in launchpad mode).

I guess we would have to publish a package on npm. Pros are the sources are in m-c, it's not a bundle so it's easy to debug at runtime, no bundle drops to do, always works with the version of devtools in m-c. Cons are we still have code duplication, won't help with external contributions.

juliandescottes commented 6 years ago

Since https://bugzilla.mozilla.org/show_bug.cgi?id=1406415 has been closed for now, there is no immediate reason to keep this RFC open.

We discussed it a bit yesterday with @nchevobbe @ochameau and @jasonLaster . We hope that a syncing solution allowing us to move the source files back to m-c can be ready soon-ish, so that when the need for shared components between m-c and GitHub projects comes again, we can avoid having a new bundle to release.

Not directly related, but we might try to move the components used by the debugger (reps, tree, source-maps etc...) directly in the debugger.html repo, and make the debugger directly depend on them rather than fetching them through npm. This would simplify the development on those components and would also simplify the dependency management, since the debugger would always be compatible with the latest versions of reps, tree etc...

juliandescottes commented 6 years ago

Closing this RFC for the moment. If the requirement comes back for other component and we don't have a better solution for GitHub?m-c workflows at that moment, we will discuss again.