Skip to content

Conversation

@Kaikina
Copy link

@Kaikina Kaikina commented Jan 26, 2026

Questions Answers
Description? FO Javascript is loaded on every FO page, BO javascript is loaded on every BO page. This PR aims to improve JS loading. On BO, JS is loaded only when loading the configuration page of the module inside getContent() method. For FO, we have two logics. First, for the customer account part, as we use a custom front controller, we use the setMedia() function to register our javascript with the defer attribute. For the product page form, we include the JS directly from the template with a defer attribute.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? NA
How to test? NA

Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

I think this is not in line with Prestashop standard. You are forcing a javascript library to be loaded outside the CCC package, outside the footer area. If jQuery is required in the bundle, it may not work due to race condition.

@ps-jarvis ps-jarvis added the waiting for author Waiting for author label Jan 26, 2026
@github-project-automation github-project-automation bot moved this to Ready for review in PR Dashboard Jan 26, 2026
@Kaikina Kaikina requested a review from Hlavtox January 26, 2026 14:57
@Kaikina
Copy link
Author

Kaikina commented Jan 26, 2026

I think this is not in line with Prestashop standard. You are forcing a javascript library to be loaded outside the CCC package, outside the footer area. If jQuery is required in the bundle, it may not work due to race condition.

About the CCC, this is true, not a fan of it, but it's true.
Race condition, I do not think as the script is defered.
Anyway, I swapped to a registerJavascript from the hook call.

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

Labels

waiting for author Waiting for author

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

3 participants