Landing CDT-independent livedev implementation

[sebaslv]

Brings in new Live Preview implementation incubated at njx/brackets-livedev2 which allows running on multiple browsers (more details on the internals at README.md). The new implementation can be used as a swap-in replacement of the current LiveDevelopment implementation.

Set livedev.multibrowser preference to true in order to make it work. If enabled, it will launch the page in your default browser when clicking on the Live Preview icon as it works today. You should then also be able to copy and paste the URL from that browser into any other browser, live edits will then update all connected browsers at once.

Main assets will land under LiveDevelopment/MultiBrowserImpl while the main module LiveDevMultiBrowser (equivalent to LiveDevelopment) will land under LiveDevelopment folder. This is probably not the more elegant choice but the decision mainly relies on these two needs:

• Keep current modules from LiveDevelopment without moving them from the original place since many extensions require them as dependencies
• Keep multi-browser implementation under this folder since it is an (still experimental) implementation of LiveDevelopment.

Changes included in LiveDevelopment/main.js allows managing both implementations. Both modules are loaded and initialized. A listener for preference changes is in charge of setting the active implementation based on pref value. Current implementation will be active and work as today by default.

There are still key things to do (launchers for particular browsers, UX/UI, multi-host support, unit/integration tests that run on different browsers, among others). Some of them will also need some previous discussion but, it is important to get this implementation landed in core so we can get some feedback from developers and also submit PRs that allows evolving it including changes that could potentially impact some areas of the underlying infrastructure.

[sebaslv]

Well, it seems that Travis is finally happy after those 3 commits. I realized that have JSLint set instead of JSHint. Please, let me know if they added some noise, I can create a new PR from a clean branch if that helps reviewing it. Thanks!

[busykai]

cc: @redmunds, @dangoor

We'd appreciate if this was given a priority to land it ASAP -- we're using this in our production and we're being left without a space to develop it further (logistically, this deprecates the extension version njx/brackets-livedev2, but it's still not in core). We cannot afford to have this hanging in limbo for a long time. I don't mean taking shortcuts, of course, just a high priority. Thanks!

[redmunds]

@busykai Both @dangoor and I are out this week, so we most likely won't get to it until next week. We need to get agreement from Product Owner @ryanstewart, but that shouldn't be a problem as he's been asking for this.
cc @pthiess @njx @sebaslv

[busykai]

@redmunds, sounds great! next week works just fine.

[redmunds]

LiveDevMultiBrowser and LiveDevelopment are inter-changeable implementations, so the public APIs must be the same. Currently, there are a lot more exports for LiveDevelopment. Some of those (e.g. LiveDevelopment.agents) can maybe be moved to unit testing section, but the rest should at least be defined and stubbed out.

[sebaslv]

@redmunds, I've already included the missing methods to align LiveDevMultiBrowser with LiveDevelopment API except for those related to agents. I think we could eventually create without big changes a mechanism for dynamically enabling/disabling remote capabilities but I think that would probably be a different PR once this implementation get merged.

[redmunds]

I'm seeing 3 Integration Code Inspection tests fail on Windows -- probably just need to sync with latest master.

[redmunds]

Running the Live Preview: MultiBrowser (experimental) - Live Development tests, they pass on Win7/Firefox, but I'm getting failures (different number every time: 3 or more) on MacOS 10.8/Safari 6.2.

[busykai]

@sebaslv, I'll look into the tests on OSX/Safari combination.

[redmunds]

Instead of livedev.multibrowser preference being a boolean, what do you think about making it an Object?

It would have to define:

• path to module
• status tooltip array
• status style array

This way, anyone could define their own Live Preview implementation. All of the LiveDevMultiBrowser code could stay as an default/extension to illustrate how to add new implementations.

[redmunds]

Thinking about this further, we'd need some way to "register live preview provider".

[sebaslv]

To make this change (in a clean way) I think I would need to change LiveDevelopment/main.js to dynamically load modules and move all the specifics things of the current implementation from main.js to LiveDevelopment module probably (eg. Inspector initialization). Not sure if that could introduce some issues with the current implementation but also with extensions that depends on it.

Making status/tooltips as a preference would probably make every developer have to know and write their values every time they want to switch among impls. The new implementation was aligned to the current UI just to make simpler its integration but, I think it is not its natural UI. It could have multiple instances running on different browsers which should be monitored/controlled somehow, even the concept of live preview session and its states would probably change. Not sure about this, I don't like current solution either, it could eventually be moved to some a config file or behind each module implementation.

Moving the new implementation back to be an extension and make it switchable with the core implementation is something I didn't try before. I assumed not to be an extension because we thought it would replace the current implementation in the long term and provide mechanisms for extending it by building custom transports, launchers, protocol methods, remote functions, etc. I didn't take into account building custom implementations because it seemed to me that building an abstraction layer on top of the current LiveDevelopment wouldn't be easy.

[redmunds]

I brought this up at team meeting today and consensus is that it's ok as it is for now.

[dangoor]

👍 I don't think we have a use case for multiple providers.. this is more of a "land behind a feature flag" kind of thing.

[redmunds]

Done with code review.

[redmunds]

I also tried out this branch with brackets-shell CEF 2171 and everything looks good!

[sebaslv]

I restored previous definitions and also changed doc for _setTransport. Thank you, @TomMalbran.

[redmunds]

@sebaslv This looks good to me. I still see the [Review Only] label -- is this ready to go into master?

[busykai]

@randy, it is ready. I will take off the [Review Only] label.

[redmunds]

Thanks for all of the work on this! Merging.

[redmunds]

@sebaslv @busykai I just noticed this console.log() statement which should be removed.

[marcelgerber]

I've just tried this the first time and well, I'm quite impressed. I wouldn't have thought this is possible and admittedly, I don't have any idea how this is implemented :) 👍
But when looking in the browsers console, there're quite a few logging lines. Are these intended?

[busykai]

Wow! Thanks to @njx, @dangoor for starting it! Thanks @redmunds for the review and feedback! Great job @sebaslv 👍 👍 👍 👍

[busykai]

@marcelgerber, yeah the message logging will be eventually turned off. However, as it is still experimental, these are needed for the developers of the feature itself.

[njx]

Thanks @busykai @sebaslv for making all this real! So great to see this landing.