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.

Sign up for GitHub

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

Jump to bottom

fix: always generate themes in frontend 'generated' directory subfolder #14144

Merged
merged 3 commits into from
Jul 8, 2022
Merged

fix: always generate themes in frontend 'generated' directory subfolder #14144

merged 3 commits into from
Jul 8, 2022

Conversation

mcollovati
Copy link
Collaborator

@mcollovati mcollovati commented Jul 6, 2022 •
edited
Loading

Description

Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

@mcollovati
fix: always generate themes in frontend 'generated' directory subfolder
d769045 
Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880
@mcollovati
Copy link
Collaborator Author

This is an alternative solution to fix #12880, by storing all generated files (excluding hilla endpoints) in ${frontendDirectory}/generated and fixing path references.
Moving generated stuff into build directory my be implemented as a new enhancement PR.

@github-actions
Copy link

github-actions bot commented Jul 6, 2022 •
edited
Loading

Unit Test Results

   904 files  +  2     904 suites  +2   56m 18s ⏱️ + 8m 50s
5 886 tests +  2  5 842 ✔️ +  1  44 💤 +1  0 ❌ ±0 
6 110 runs  +20  6 060 ✔️ +19  50 💤 +1  0 ❌ ±0 

Results for commit f01947b. ± Comparison against base commit 4aabd6a.

♻️ This comment has been updated with latest results.

@mcollovati mcollovati marked this pull request as ready for review July 6, 2022 13:15
@mcollovati mcollovati requested review from taefi and Artur- July 6, 2022 13:15
taefi
taefi reviewed Jul 7, 2022
View reviewed changes
Copy link
Contributor

@taefi taefi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider adding a test that checks some of the generated files especially theme-[theme name].generated.js is not generated under the default frontend directory anymore when there is a custom frontend directory defined?

@mcollovati
Copy link
Collaborator Author

Should we consider adding a test that checks some of the generated files especially theme-[theme name].generated.js is not generated under the default frontend directory anymore when there is a custom frontend directory defined?

It seems a good idea. I'll try to add a test for this

taefi
taefi previously approved these changes Jul 7, 2022
View reviewed changes
@mcollovati mcollovati mentioned this pull request Jul 7, 2022
Merged
9 tasks
Artur-
Artur- requested changes Jul 7, 2022
View reviewed changes
Copy link
Member

@Artur- Artur- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No major comments but if we are hardcoding the use of generated for theme files we should use the same hardcoded path also for all other generated files and drop the possibility to customize the name/location of the generated folder

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateThemeImport.java Show resolved Hide resolved
flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateWebpack.java Outdated Show resolved Hide resolved
@mcollovati
Copy link
Collaborator Author

After this change, the only generated folder that can be customized will be the target folder for Hilla endpoints.
The maven/gradle setting is generatedTsFolder.
We can eventually remove this setting and hard-code it to ${frontendDirectory}/generated in another PR, after checking usages also in Hilla repository.

@sonarcloud
Copy link

sonarcloud bot commented Jul 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mcollovati mcollovati requested review from Artur- and taefi July 8, 2022 06:14
@mcollovati mcollovati merged commit ac92cfe into master Jul 8, 2022
@mcollovati mcollovati deleted the issues/12880_custom_frontend_folder_2 branch July 8, 2022 08:27
@mcollovati mcollovati mentioned this pull request Jul 9, 2022
Closed
9 tasks
vaadin-bot pushed a commit that referenced this pull request Jul 9, 2022
@mcollovati @vaadin-bot
fix: always generate themes in frontend 'generated' directory subfold…
ea57f58 
…er (#14144)

Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880
vaadin-bot pushed a commit that referenced this pull request Jul 9, 2022
@mcollovati @vaadin-bot
fix: always generate themes in frontend 'generated' directory subfold…
72deeff 
…er (#14144)

Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880
mcollovati pushed a commit that referenced this pull request Jul 11, 2022
@vaadin-bot
fix: always generate themes in frontend 'generated' directory subfold…
998e208 
…er (#14144) (CP: 23.0) (#14156)

Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880
mcollovati pushed a commit that referenced this pull request Jul 11, 2022
@vaadin-bot
fix: always generate themes in frontend 'generated' directory subfold…
9f545e5 
…er (#14144) (CP: 23.1) (#14155)

Forces generated themes files to be stored in 'generated' folder under
frontend directory, instead of using the 'generatedTsFolder' setting.
Also fixes relative paths assuming frontend directory to be './frontend',
and maven and gradle plugins to clean the 'frontendDirectory/generated' folder.

Fixes #12880
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.2.0.

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

@taefi taefi taefi approved these changes

@Artur- Artur- Artur- approved these changes

Assignees
No one assigned
Labels
cherry-picked-23.0 cherry-picked-23.1 Released with Vaadin 23.2.0 +0.0.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

custom frontend folder does not work
4 participants
@mcollovati @vaadin-bot @Artur- @taefi