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

Bundle is not rebuilt if you change files in the theme/components folder #16407

Closed
eriklumme opened this issue Mar 24, 2023 · 13 comments · Fixed by #17776
Closed

Bundle is not rebuilt if you change files in the theme/components folder #16407

eriklumme opened this issue Mar 24, 2023 · 13 comments · Fixed by #17776

Comments

@eriklumme
Copy link

eriklumme commented Mar 24, 2023

Description of the bug

When your application has a theme folder, you can create files inside the components subfolder to override component styles.

These are not picked up unless there is a @CssImport in the application, or something else that triggers the frontend build.

I assume that this is an issue with using precompiled resources unless any frontend resources are modified, but it fails to notice that it's needed when overriding component styles.

Expected behavior

Adding files to the theme components folder should work regardless of using CSS imports or not.

Minimal reproducible example

  1. Go to start.vaadin.com
  2. Download
  3. Into ./themes/myapp/components, create a file vaadin-text-field.css with the following contents.
[part="input-field"] {
    background: blue;
}
  1. Run the application with ./mvnw
  2. The background has not changed.
  3. Into the Application.java class, add the following @CssImport:
    @CssImport("./themes/myapp/styles.css")
  4. Restart the application
  5. The application now runs the frontend build, creating node_modules etc., and once completed, the background of the text field is blue.

Versions

  • Vaadin / Flow version: 24.0.0, 24.0.1
  • Java version: 19.0.1
  • OS version: Mac OSX 12.5.1
@eriklumme
Copy link
Author

This also manifests itself in another way:

  1. In a new project, create a file ./frontend/test.css with this content
body {
    background: red !important;
}
  1. Add a @CssImport to the Application class

@CssImport("./test.css")

  1. Run the project ./mvnw, and a part of the page will have a red background, as expected.
  2. Remove the @CssImport and restart.
  3. The red background will still be there, even if you remove the stylesheet. To remove the background, you must delete some generated files, or reapply the @CssImport with an empty CSS-file, and then remove it again.

@Artur- Artur- changed the title Component theme not picked up unless there is a @CssImport in the application Bundle is not rebuilt if you change files in the theme/components folder Mar 27, 2023
@czp13 czp13 added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Mar 28, 2023
@mshabarov mshabarov added this to Parking lot - under consideration in OLD Vaadin Flow ongoing work (Vaadin 10+) May 2, 2023
@mshabarov mshabarov moved this from Inbox (needs triage) to Maybe in Vaadin Flow enhancements backlog (Vaadin 10+) May 2, 2023
@mshabarov
Copy link
Contributor

I'd consider this as an enhancement. When we introduced pre-compiled dev bundle feature, we relied on a new ::part API for styling components, so the changes in components weren't included into bundle rebuild checker. In the same time, detecting changes in my-theme/components and re-compile the the dev bundle is not a big deal in my opinion, this can be done in a similar way as for JS/TS files in the frontend folder. I think it makes sense to plan it for V24.2, meanwhile I recommend to use frontend.hotdeploy=true when you style the components stylesheets in the my-theme/components.

@mshabarov
Copy link
Contributor

The problem described here #16407 (comment) looks as a bug to me. I suggest to make a new ticket to track it, because it has nothing to do with my-theme/components styles.

@Freeman656
Copy link

I'm in the same case, I have no @CssImport, but I'm using the components folder for fine tuning the scrollbars of a grid.
I guess we can still use @CssImport with themeFor for this purpose, but as it is deprecated, the best way is to get the components folder to work properly.
Are you able to include it in an ealier version (maybe next 21.1.x ?).
Thanks !

@rolfsmeds
Copy link

@Freeman656 note that, as @mshabarov mentioned, the workaround is to disable the bundle (no need to resort to @CssImport) :

meanwhile I recommend to use frontend.hotdeploy=true when you style the components stylesheets in the my-theme/components.

@mshabarov mshabarov moved this from Parking lot - under consideration to Product backlog in OLD Vaadin Flow ongoing work (Vaadin 10+) Aug 14, 2023
@mshabarov mshabarov moved this from Maybe to Next for Dev. Team in Vaadin Flow enhancements backlog (Vaadin 10+) Aug 14, 2023
@mshabarov
Copy link
Contributor

This issue can be seen in production mode as well, Vaadin ignores the styles in themes/my-theme/components at the moment and tells:

[INFO] Checking if a production mode bundle build is needed
[INFO] A production mode bundle build is not needed

having no styles in the production bundle.

While for development mode the workaround is to set vaadin.frontend.hotdeploy = true, for production mode the same can be achieved by forcing production build always:
-Dvaadin.force.production.build=true or

                    <plugin>
                        <groupId>com.vaadin</groupId>
                        <artifactId>vaadin-maven-plugin</artifactId>
                        <version>${vaadin.version}</version>
                        <executions>
                            <execution>
                                <goals>
                                    <goal>build-frontend</goal>
                                </goals>
                                <phase>compile</phase>
                            </execution>
                        </executions>
                        <configuration>
                            <forceProductionBuild>true</forceProductionBuild>
                        </configuration>
                    </plugin>

@mshabarov
Copy link
Contributor

I'd like to prioritise this as a high priority bug, which we have to fix in upcoming Vaadin 24.2 and hopefully pick then into 24.1.

@mcollovati mcollovati self-assigned this Sep 29, 2023
@mcollovati
Copy link
Collaborator

In the documentation, we state that the components sub-folder isn’t compatible with the pre-compiled front-end bundle.

image

I wonder if we should always require a build of the bundle if there's at least on CSS file in the <themeDirectory>/components (or parent theme)

@mcollovati
Copy link
Collaborator

For the issue in #16407 (comment) I can confirm that when we check if a bundle is needed, we do not consider if the bundle stats contain an entry that is not required anymore.

@mshabarov
Copy link
Contributor

I wonder if we should always require a build of the bundle if there's at least on CSS file in the /components (or parent theme)

Yes, whenever any of the files in /components is updated or added, Flow should re-build the bundle.

@mshabarov
Copy link
Contributor

For the issue in #16407 (comment) I can confirm that when we check if a bundle is needed, we do not consider if the bundle stats contain an entry that is not required anymore.

I propose to address this separately (as an another patch), this isn't so severe, like a previous one, but nevertheless would be nice to have.

mcollovati added a commit that referenced this issue Oct 5, 2023
The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407
mshabarov pushed a commit that referenced this issue Oct 6, 2023
* fix: handle shadow DOM stylesheets in production and dev bundle

The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407

* apply review suggestions

* apply review suggestions
vaadin-bot pushed a commit that referenced this issue Oct 6, 2023
* fix: handle shadow DOM stylesheets in production and dev bundle

The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407

* apply review suggestions

* apply review suggestions
vaadin-bot pushed a commit that referenced this issue Oct 6, 2023
* fix: handle shadow DOM stylesheets in production and dev bundle

The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407

* apply review suggestions

* apply review suggestions
vaadin-bot added a commit that referenced this issue Oct 6, 2023
…) (CP: 24.1) (#17798)

* fix: handle shadow DOM stylesheets in production and dev bundle (#17776)

* fix: handle shadow DOM stylesheets in production and dev bundle

The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407

* apply review suggestions

* apply review suggestions

* fixed parent pom version

---------

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this issue Oct 6, 2023
…) (CP: 24.2) (#17797)

* fix: handle shadow DOM stylesheets in production and dev bundle (#17776)

* fix: handle shadow DOM stylesheets in production and dev bundle

The legacy shadow DOM stylesheets that can potentially be present in the
'theme/<themeName>/components/' folder are not considered when deciding
if a new bundle needs to be created, so the application may miss custom
components styles.
This change checks for existence of theme components CSS, and it triggers
a bundle rebuild if any is found.

Fixes #16407

* apply review suggestions

* apply review suggestions

* fixed parent pom version

---------

Co-authored-by: Marco Collovati <marco@vaadin.com>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.2.0.rc1 and is also targeting the upcoming stable 24.2.0 version.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.0.alpha1 and is also targeting the upcoming stable 24.3.0 version.

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