Consolidate frontend #9915
Consolidate frontend #9915
Conversation
|
Will I'm thinking of a developer who just wants to develop backend: |
|
No. One will no longer be able to work on backend stuff without compiling the frontend once. To make it easier to compile I'll include a |
|
We're not going to merge these PRs before 0.56 release. |
|
Will one be able to work on backend stuff if they don't have I thought the intent of this PR was to decouple FE from the BE? |
|
One can always work on any backend stuff without loading the frontend. You will just need to depend on tests instead of playing with it in the UI |
|
Brainstorming idea: What if we published the prebuilt polymer files on PyPI as an independent package? It could then be registered in frontend component using the existing REQUIREMENTS system. If the frontend component isn't configured with It's obviously messy that we'd have a PyPI package that only contains the polymer code, but we already include the frontend files in our current package. This would just be further separation of the two repos. |
|
Tbh, I would actually prefer to have some way to get the system running without first fighting quite a bit to get the front-end stuff to compile & working. However, it feels wrong to ship some auxiliary binary package to pypi. Therefore I would encourage that we need find a simple way to get UI bits compiled in a simple manner (wrt. pip install x) instead of going for dependency hacks on some pypi snapshot from the frontend. In my opinion providing an option to use docker for this is not enough. Just my 0.02e, I applaud the idea of this PR completely so do not take this as a disagreement :-) |
|
I'm mainly concerned with raising the barrier to entry for contribution, and personally would prefer some channel to ship the precompiled frontend to developers. If we ballpark by issue/PR count, over 95% of our contributions are to the backend. If people aren't actively developing the frontend, I don't see why we need everyone to rebuild it from source themselves. Especially since we are attempting to isolate the frontend from the backend, it makes sense that we'd move toward shipping it as it's own product. Docker certainly will make building the frontend easier, but it's a big additional hurdle to people who are often starting their first foray into open source, and are not familiar with all of the tooling. I just want to make sure we carefully consider the ramifications of the final solution on casual developers. |
|
I like the idea of publishing it as a pypi package a lot! |
|
I've added initial work to convert it to a Python package: home-assistant/frontend@ef7911c |
| router.add_route( | ||
| 'get', '/{}'.format(self.frontend_url_path), index_view.get) | ||
| router.add_route( | ||
| 'get', '/{}/{{extra:.+}}'.format(self.frontend_url_path), index_view.get) |
houndci-bot
Oct 22, 2017
line too long (85 > 79 characters)
line too long (85 > 79 characters)
| if url not in _REGISTERED_COMPONENTS: | ||
| hass.http.register_static_path(url, path) | ||
| _REGISTERED_COMPONENTS.add(url) | ||
| @callback |
houndci-bot
Oct 22, 2017
too many blank lines (2)
too many blank lines (2)
|
@armills last commit has changed it to importing the frontend from a PyPi package. It will not use the PyPi package when you pass in a path to the polymer repo: frontend:
development_repo: ../home-assistant-polymerWill still need to fix the tests. |
| os.path.join(FINAL_PATH, "robots.txt")) | ||
| hass.http.register_static_path("/static", FINAL_PATH) | ||
| if hass.http.development: | ||
| hass.http.register_static_path("/home-assistant-polymer", POLYMER_PATH) |
andrey-git
Oct 22, 2017
Contributor
You are now registering both dev & prod paths. Is this on purpose?
You are now registering both dev & prod paths. Is this on purpose?
| @@ -11,13 +11,6 @@ echo "Bootstrapping frontend..." | |||
| git submodule update --init | |||
| cd homeassistant/components/frontend/www_static/home-assistant-polymer | |||
andrey-git
Oct 22, 2017
Contributor
This is now homeassistant/components/frontend/home-assistant-polymer right?
This is now homeassistant/components/frontend/home-assistant-polymer right?
emlove
Oct 22, 2017
Contributor
I thought we're getting rid of the submodule completely, so devs will have this checked out at an arbitrary location they configure. I think we'd want to just remove script/bootstrap_frontend, and the docs will explain how to point to the other repo for development.
I thought we're getting rid of the submodule completely, so devs will have this checked out at an arbitrary location they configure. I think we'd want to just remove script/bootstrap_frontend, and the docs will explain how to point to the other repo for development.
| python3 setup.py sdist bdist_wheel upload | ||
| script/build_frontend | ||
|
|
||
| # python3 setup.py sdist bdist_wheel upload |
andrey-git
Oct 22, 2017
Contributor
Should it be uncommented?
Should it be uncommented?
| @@ -294,26 +300,6 @@ def reload_themes(_): | |||
| descriptions[SERVICE_RELOAD_THEMES]) | |||
|
|
|||
|
|
|||
| class BootstrapView(HomeAssistantView): | |||
andrey-git
Oct 22, 2017
Contributor
Why did you remove it?
Why did you remove it?
| DATA_THEMES = 'frontend_themes' | ||
| DATA_DEFAULT_THEME = 'frontend_default_theme' | ||
| DEFAULT_THEME = 'default' | ||
|
|
||
| PRIMARY_COLOR = 'primary-color' | ||
|
|
||
| # To keep track we don't register a component twice (gives a warning) | ||
| _REGISTERED_COMPONENTS = set() | ||
| # _REGISTERED_COMPONENTS = set() |
andrey-git
Oct 22, 2017
Contributor
This, and the comment above is no longer needed, right?
This, and the comment above is no longer needed, right?
|
Looks great! Now, if I make a FE change and I would like to compile it and check that compiled code works - how do I do that? |
|
Good question So one way I have been doing it now is to run I do realize that that's suboptimal. Guess we need another config option to load the compiled version from the repo. Shouldn't be too hard but for the sake of getting this monster merged, can we do that in another PR? |
|
Yeah this shouldn't block merging this PR Maybe put a sticky message (not only regarding compiling FE, but about the whole new dev process) in #dev_frontend and #dev chat until this is fixed and new documentation is pushed to the site. |
|
Also, brace yourselves for recommits of the polymer submodule in every PR. |
| 'url_path': 'router' | ||
| } | ||
|
|
||
| assert self.hass.data[frontend.DATA_PANELS].get('weather') == { | ||
| assert self.hass.data[frontend.DATA_PANELS].get('weather').as_dict() == { |
houndci-bot
Oct 22, 2017
line too long (81 > 79 characters)
line too long (81 > 79 characters)
| @@ -53,20 +53,20 @@ def test_correct_config(self): | |||
| }, | |||
| }) | |||
|
|
|||
| assert self.hass.data[frontend.DATA_PANELS].get('router') == { | |||
| assert self.hass.data[frontend.DATA_PANELS].get('router').as_dict() == { | |||
houndci-bot
Oct 22, 2017
line too long (80 > 79 characters)
line too long (80 > 79 characters)
| from homeassistant import setup | ||
| from homeassistant.components import panel_custom | ||
|
|
||
| from tests.common import get_test_home_assistant | ||
| from tests.common import get_test_home_assistant, mock_coro, mock_component |
houndci-bot
Oct 22, 2017
'tests.common.get_test_home_assistant' imported but unused
'tests.common.get_test_home_assistant' imported but unused
| hass.data[DATA_PANELS].pop(self.frontend_url_path) | ||
|
|
||
|
|
||
| self.webcomponent_url = \ |
houndci-bot
Oct 22, 2017
too many blank lines (2)
too many blank lines (2)
| 'url_path': 'router' | ||
| } | ||
|
|
||
| assert self.hass.data[frontend.DATA_PANELS].get('weather') == { | ||
| assert self.hass.data[frontend.DATA_PANELS].get('weather').as_dict() == { |
houndci-bot
Oct 22, 2017
line too long (81 > 79 characters)
line too long (81 > 79 characters)
| @@ -53,20 +53,20 @@ def test_correct_config(self): | |||
| }, | |||
| }) | |||
|
|
|||
| assert self.hass.data[frontend.DATA_PANELS].get('router') == { | |||
| assert self.hass.data[frontend.DATA_PANELS].get('router').as_dict() == { | |||
houndci-bot
Oct 22, 2017
line too long (80 > 79 characters)
line too long (80 > 79 characters)
| from homeassistant import setup | ||
| from homeassistant.components import panel_custom | ||
|
|
||
| from tests.common import get_test_home_assistant | ||
| from tests.common import get_test_home_assistant, mock_coro, mock_component |
houndci-bot
Oct 22, 2017
'tests.common.get_test_home_assistant' imported but unused
'tests.common.get_test_home_assistant' imported but unused
| hass.data[DATA_PANELS].pop(self.frontend_url_path) | ||
|
|
||
|
|
||
| self.webcomponent_url = \ |
houndci-bot
Oct 22, 2017
too many blank lines (2)
too many blank lines (2)
|
Fixed the tests, this is ready for review. |
| register_built_in_panel(hass, panel) | ||
| # Finalize registration of panels that registered before frontend was setup | ||
| # This includes the built-in panels from line above. | ||
| yield from asyncio.wait([finalize_panel(panel) for panel |
emlove
Oct 22, 2017
Contributor
We should update hass.data[DATA_FINALIZE_PANEL] before this in case any panels register while this is waiting.
We should update hass.data[DATA_FINALIZE_PANEL] before this in case any panels register while this is waiting.
| @@ -11,13 +11,6 @@ echo "Bootstrapping frontend..." | |||
| git submodule update --init | |||
| cd homeassistant/components/frontend/www_static/home-assistant-polymer | |||
emlove
Oct 22, 2017
Contributor
I thought we're getting rid of the submodule completely, so devs will have this checked out at an arbitrary location they configure. I think we'd want to just remove script/bootstrap_frontend, and the docs will explain how to point to the other repo for development.
I thought we're getting rid of the submodule completely, so devs will have this checked out at an arbitrary location they configure. I think we'd want to just remove script/bootstrap_frontend, and the docs will explain how to point to the other repo for development.
|
@armills @andrey-git shall we merge this? |
|
Looks great! This is a huge step towards decoupling the frontend! |
is still used for cache control. |
|
When I want to run with FE dev mode I set It seems so, since |
|
Yes, build the frontend once. Removing Dev option from http component is on todo |
Description:
This moves all frontend related static assets to the polymer repo. Building scripts will also be moved to the repo. We will no longer contain a version of the frontend ready to go when doing a git checkout. Frontend will still be included in the PyPi package.
This PR is the first step. Probably some more cleanup PRs will follow after this.
Polymer PR: home-assistant/frontend#468
Breaking change:
Developers only: frontend has been refactored. The method
register_panelhas been turned into a coroutine function calledasync_register_panel. The parameterurl_pathhas been renamed tofrontend_url_path. For frontend, development, you no longer passdevelopment: 1to the http component but instead configure the frontend component to be in development mode by pointing it at a local checkout of the Polymer repo: