Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom theme live-reload doesn't work for newly added stylesheet files #10544

Closed
mshabarov opened this issue Apr 1, 2021 · 8 comments
Closed

Comments

@mshabarov
Copy link
Contributor

Description of the bug / feature

Since V19.0.0.alpha5 the custom theme live-reload (webpack re-compilation) is not being triggered upon adding a new stylesheet files (or any other files) into frontend/themes/my-custom-theme/ (including components folder).
It works fine with V19.0.0.alpha4.

Minimal reproducible example

  1. Go to start.vaadin.com
  2. Choose Vaadin 19 in the Settings tab
  3. Download the project and run mvn
  4. Add a new file vaadin-button.css (or any other name) to the {project.root}/frontend/themes/myapp/components/
  5. Notice no webpack re-compilation in the console logs and no app restart
  6. Add a new file foo.css (or any other name) to the {project.root}/frontend/themes/myapp/
  7. Notice no webpack re-compilation in the console logs and no app restart
  8. Change vaadin.version to 19.0.0.alpha4 in pom.xml, delete node_modules and any generated files in project root, then run mvn clean spring-boot:run
  9. Do steps 4, 6 and check that the application restarts upon adding the files.

Expected behavior

Webpack re-compiles and the application restarted, applying a newly added styles.

Actual behavior

Nothing happens upon adding a new file.

Versions:

- Vaadin / Flow version: V19.0.2, Flow 6.0.3
- Java version: 8
- OS version: Mac OS
- Browser version (if applicable): Chrome 89.0.4389.90
- Application Server (if applicable): spring-boot
- IDE (if applicable): IntelliJ IDEA 2020.3
@mshabarov mshabarov added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Apr 1, 2021
@mshabarov mshabarov moved this from Needs triage to P1 - High priority in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Apr 1, 2021
mshabarov added a commit that referenced this issue Apr 7, 2021
Extract the custom theme name from frontend generated folder and skips theme handling when the theme generated file is being created/updated.

Related-to #10451, #10544
@mshabarov
Copy link
Contributor Author

When the generated theme files were moved into separate folder, this code did not changed (webpack config file):

const themeName = extractThemeName(flowFrontendThemesFolder);

It should have extracted the theme name from frontendGeneratedFolder, not from flowFrontendThemesFolder.
As a result, the watch and live reload plugins were not activated at all, because the theme name was empty.

This also shows how necessary it is to maintain the tests in a good condition.
Theme live reload test was disabled because of flakiness, while the change was done and didn't remind to change this folder either.

@mshabarov mshabarov self-assigned this Apr 8, 2021
@mshabarov mshabarov moved this from P1 - High priority to WIP in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Apr 8, 2021
@pleku
Copy link
Contributor

pleku commented Apr 9, 2021

Add a new file foo.css (or any other name) to the {project.root}/frontend/themes/myapp/

@mshabarov I'd like to note that this is not even supposed to work - you have to reference the added file in styles.css with @import url('foo.css'); and then live reload works with v19.0.3

@mshabarov
Copy link
Contributor Author

Good point! The watch folder is a whole theme folder, and thus adding any file to theme folder root triggers the live reload at the moment.
ExtraWatchWebpackPlugin should be modified so as to watch frontend/themes/my-theme/components only.
Other subfolders/resource files/styles added into my-theme shouldn't trigger any live reload until they are referenced from styles.css.

This comment forces me to think what if we switch the theme on fly...then the live-reload for components folder in the new theme folder won't works because it was not set up initially.

@pleku
Copy link
Contributor

pleku commented Apr 9, 2021

This comment forces me to think what if we switch the theme on fly...then the live-reload for components folder in the new theme folder won't works because it was not set up initially.

So can the live reload expand the watcher scope but be made context aware - I mean that we would watch everything under /themes/ but skip any action for the changed files for the non-active-theme folder ? Or was that tried already and was problematic ?

@mshabarov
Copy link
Contributor Author

I mean that we would watch everything under /themes/ but skip any action for the changed files for the non-active-theme folder ?

Well, the problem is that the theme live reload plugin should know what is the current theme name to be able to exclude the changes in non-active folder.
Thus, either this plugin or extended watch plugin should be somehow re-configured on the fly.
I haven't researched it deeply yet.

mshabarov added a commit that referenced this issue Apr 12, 2021
Extracts the custom theme name from frontend generated folder and skips theme handling when the theme generated file is being created/updated.

Related-to #10451, #10544
mshabarov added a commit that referenced this issue Apr 12, 2021
Watch the components folder for component styles update.
Other folders or CSS files except 'styles.css' should be referenced from `styles.css` anyway, so no need to watch them.

Related-to #10544
mshabarov added a commit that referenced this issue Apr 12, 2021
…10622)

Extracts the custom theme name from frontend generated folder and skips theme handling when the theme generated file is being created/updated.

Related-to #10451, #10544
caalador pushed a commit that referenced this issue Apr 14, 2021
Watch the components folder for component styles update.
Other folders or CSS files except 'styles.css' should be referenced from `styles.css` anyway, so no need to watch them.

Related-to #10544
ZheSun88 pushed a commit that referenced this issue Apr 14, 2021
…10622)

Extracts the custom theme name from frontend generated folder and skips theme handling when the theme generated file is being created/updated.

Related-to #10451, #10544
mshabarov added a commit that referenced this issue Apr 15, 2021
…10622) (#10647)

Extracts the custom theme name from frontend generated folder and skips theme handling when the theme generated file is being created/updated.

Related-to #10451, #10544

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
@mshabarov
Copy link
Contributor Author

This comment forces me to think what if we switch the theme on fly...then the live-reload for components folder in the new theme folder won't works because it was not set up initially.

Created a separate issue for it #10680

OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Apr 15, 2021
@mshabarov mshabarov reopened this Apr 15, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from Closed to Needs triage Apr 15, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from Needs triage to Closed Apr 15, 2021
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 19.0.5. For prerelease versions, it will be included in its final version.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 14.6.0.beta2. For prerelease versions, it will be included in its final version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment