-
Notifications
You must be signed in to change notification settings - Fork 1
LPD-52709 Part 6: language request handler #5219
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
base: master
Are you sure you want to change the base?
Conversation
…r internal ESM files We need to use language labels modules for caching to be feasible so we will split .js modules in two parts: code itself and labels in a different file, thus we don't need the LanguageFilter to kick in any more. This will make the use of ETags for .js files not necessary any more. Note however that legacy /combo URLs are still routed through the filter. In any case this should only be used for YUI code, not for ESM.
We will integrate all resources managed by frontend-js-web in a single filter that will delegate to request helpers in the next commit. This is to be able to centralize all frontend resources management (CSS, JS, source maps, etc) in one place and reuse strategies for serving them.
… changes Since portlet JS files are not intended to be ESM format and outside of that context we cannot use the new translation architecture based on all.js files, I'm putting back again portlet JS files translation. It essentially works the same as tokenized CSS files. Note that translated JS files will not be infinitely cacheable but time-based, for obvious reasons.
This need was discovered (for JS files) while implementing the previous commit so I'm adding it for CSS files too.
…rces any more We never published anything related to supporting it, so it can be considered an internal.
|
CI is automatically triggering the following test suites:
|
|
Test suite sf has been triggered on http://test-1-32 |
Summary of ChangesHello @izaera, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors Liferay's frontend resource handling to introduce a robust mechanism for translating JavaScript files, particularly for portlets. It moves away from an older servlet-based system to a more integrated resource handler approach, ensuring that JavaScript assets can be localized and efficiently cached. The changes also include updates to the build process to incorporate content hashing for better cache invalidation and new configuration options for fine-tuning caching behavior of translated modules. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
ci:test:relevant |
|
Test suite relevant has been triggered on http://test-1-35 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant and well-architected refactoring of how JavaScript language translations are handled. The shift from a dedicated servlet to a more flexible filter-and-handler mechanism is a great improvement. The new capability to automatically translate JavaScript files containing Liferay.Language.get by adding request parameters is a clean solution. The changes are extensive, touching build scripts, configuration, core portal logic, and tests, and they appear to be moving in a very positive direction. I've identified a few potential issues in the implementation that should be addressed to ensure correctness and robustness. My comments highlight a bug in the Gradle build script, a potential for silent failures in resource loading, and a logic issue in handling multiple portlet resource bundles.
modules/apps/frontend-js/frontend-js-loader-modules-extender/build.gradle
Show resolved
Hide resolved
...eb/src/main/java/com/liferay/frontend/js/web/internal/resource/LanguageFrontendResource.java
Show resolved
Hide resolved
...eray/frontend/js/web/internal/resource/handler/JavaScriptFrontendResourceRequestHandler.java
Show resolved
Hide resolved
✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutesRan com.liferay.source.formatter at released version 1.0.1554. Click here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-52709-6 1 Successful Jobs:For more details click here. |
|
Jenkins Build:test-portal-source-format#11495 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#5219 Testray Routine:EE Pull Request Testray Build ID: 385690498Testray Importer:test-portal-source-format#11495 |
✔️ ci:test:stable - 12 out of 12 jobs passed❌ ci:test:relevant - 19 out of 50 jobs passed in 2 hours 32 minutesClick here for more details.Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: 975925f19730f06edb987e17df2fd656e6393736 ci:test:stable - 12 out of 12 jobs PASSED12 Successful Jobs:ci:test:relevant - 19 out of 50 jobs PASSED31 Failed Jobs:
19 Successful Jobs:For more details click here.Failures unique to this pull:
For upstream results, click here.Test bundle downloads: |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#8127 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#5219 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - izaera > liferay-frontend - PR#5219 - 2025-12-09[07:14:04] Testray Build ID: 385769796Testray Importer:test-portal-acceptance-pullrequest(master)#8127 |
|
ci:report:385769796 |
|
Test suite default has been triggered on http://test-1-37 |
Build completed.Testray CSV has been generated successfully for testrayBuildID: 385769796. Job Link: generate-testray-csv Testray CSV Link: testray-results-385769796.csv |
Resend of brianchandotcom#168320 with extra code to avoid breaking changes due to .js files not being translated for portlets.