fixes requireJS mixins racecondition with require calls in dom
Description (*)
Fixes the race condition with DOM require calls parsing before require(['mixins'],{...}); replacing require function.
Resolves mixins not applying.
Ensures jquery-mixin from security https://github.com/magento/security-package/blob/develop/ReCaptchaWebapiUi/view/frontend/requirejs-config.js#L10 is mixed in before jquery called
How to Confirm You Have This Issue:
add to custom block output in top of page of cart closest to body opening tag, visit empty cart page /checkout/cart/index
context should be from wrapper (mixin applied) not from customer-data (mixin not applied)
<script>
require(['jquery', 'Magento_Customer/js/customer-data'], function($, customerData) {
'use strict';
console.log("context from mixin?: %O", customerData.getExpiredSectionNames);
});
</script>
Original Process: requirejs makeRequire calls nexTick (setTimeout) freeing thread to parse DOM requirejs/mixins.js loads and define call is made DOM require calls race condition - if calls happen require context is from requirejs/require.js and not mixins.js requirejs/mixins.js require fired and now instantiating and replacing require call context to mixins.js
Changes:
Moves requirejs-config.js before mixins.js ensures configurations are loaded
Remove context.nextTick wrap from makeRequire return localRequire function (this was freeing thread to DOM) **
Remove redundant intakeDefines();
Move require calls out of require-configs. Only one is for jquery no conflict. Remove from https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Theme/view/base/requirejs-config.js#L87 and move to end of require(['mixins], function(..) { ... {beforeClosingtag} }); ensures mixin replaces require function so jquery-mixin is properly applied
**checkLoaded() has all the setTimeout functionality we need https://github.com/magento/magento2/blob/9124539dd5d26ea31dcda8608071582d2bfbde1a/lib/web/requirejs/require.js#L717
See more information in my comment on the issue this PR should fix below.
Related Pull Requests
Fixed Issues (if relevant)
- Fixes magento/magento2#33593
- Properly Fixes magento/magento2#31920
- Addresses #38274 (config output may need hoisting see my comment in ticket)
- Should also fix magento/baler#23
Manual testing scenarios (*)
See the above issue for steps to reproduce mixins not loading issue caused from this race condition additionally can
- increase nextTick setTimeout value in requirejs.js to a larger number,increment more if needed to replicate consistently
- reload page
- verify dom require calls are parsed before require(['mixins']
Questions or comments
Contribution checklist (*)
- [x] Pull request has a meaningful description of its purpose
- [x] All commits are accompanied by meaningful commit messages
- [ ] All new or changed code is covered with unit/integration tests (if applicable)
- [ ] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
- [ ] All automated tests passed successfully (all builds are green)
Hi @AndresInSpace. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:
-
@magento give me test instance- deploy test instance based on PR changes -
@magento give me 2.4-develop instance- deploy vanilla Magento instance
:exclamation: Automated tests can be triggered manually with an appropriate comment:
-
@magento run all tests- run or re-run all required tests against the PR changes -
@magento run <test-build(s)>- run or re-run specific test build(s) For example:@magento run Unit Tests
<test-build(s)> is a comma-separated list of build names.
Allowed build names are:
-
Database Compare -
Functional Tests CE -
Functional Tests EE -
Functional Tests B2B -
Integration Tests -
Magento Health Index -
Sample Data Tests CE -
Sample Data Tests EE -
Sample Data Tests B2B -
Static Tests -
Unit Tests -
WebAPI Tests -
Semantic Version Checker
You can find more information about the builds here :information_source: Run only required test builds during development. Run all test builds before sending your pull request for review.
For more details, review the Code Contributions documentation. Join Magento Community Engineering Slack and ask your questions in #github channel.
@magento give me test instance
On Thu, Aug 22, 2024, 10:16 PM m2-assistant[bot] @.***> wrote:
Hi @AndresInSpace https://github.com/AndresInSpace. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment. Add the comment under your pull request to deploy test or vanilla Magento instance:
- @magento give me test instance - deploy test instance based on PR changes
- @magento give me 2.4-develop instance - deploy vanilla Magento instance
❗ Automated tests can be triggered manually with an appropriate comment:
- @magento run all tests - run or re-run all required tests against the PR changes
- @magento run <test-build(s)> - run or re-run specific test build(s) For example: @magento run Unit Tests
<test-build(s)> is a comma-separated list of build names. Allowed build names are:
- Database Compare
- Functional Tests CE
- Functional Tests EE
- Functional Tests B2B
- Integration Tests
- Magento Health Index
- Sample Data Tests CE
- Sample Data Tests EE
- Sample Data Tests B2B
- Static Tests
- Unit Tests
- WebAPI Tests
- Semantic Version Checker
You can find more information about the builds here https://github.com/magento/magento2/wiki/Magento-Automated-Testing ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.
For more details, review the Code Contributions https://developer.adobe.com/commerce/contributor/guides/code-contributions/ documentation. Join Magento Community Engineering Slack https://developer.adobe.com/open/magento/slack and ask your questions in #github https://magentocommeng.slack.com/messages/C7KB93M32 channel.
— Reply to this email directly, view it on GitHub https://github.com/magento/magento2/pull/39097#issuecomment-2306059135, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADNQ47QTMLXF2NWF4BSYBDDZS2LQDAVCNFSM6AAAAABM7IWNX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBWGA2TSMJTGU . You are receiving this because you were mentioned.Message ID: @.***>
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering Admin access: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering/admin_9a90 Login: 9dd942e3 Password: bf5d160ddea5
@magento give me 2.4-develop instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering Admin access: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering/admin_56cc Login: c47b2027 Password: e1500fe5aee1
Appears resolved based on testing customer-data in a require statement added via home block so it is very close to opening body tag in order to trigger the error.
<script>
require(['jquery', 'Magento_Customer/js/customer-data'], function($, customerData) {
'use strict';
console.log("context from mixin?: %O", customerData.getExpiredSectionNames);
});
</script>
View console log F12 & Reload w cache disabled or ctrl+f5 hard refresh chrome on windows
Vanilla instance shows customerData.getExpiredSectionNames from customer-data.js not the mixin override
PR Instance correctly shows customerData.getExpiredSectionNames from wrapper, as mixin extended it.
@magento run all tests
@magento give me test instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering Admin access: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering/admin_64bd Login: 7a75a37a Password: 9984b8486411
@magento give me test instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering Admin access: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering/admin_13df Login: 4b195ca5 Password: e2320ead2d47
@magento give me 2.4-develop instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering Admin access: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering/admin_a159 Login: 728298b6 Password: c368314a96b0
@magento give me 2.4-develop instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
@magento give me test instance
Hi @AndresInSpace. Thank you for your request. I'm working on Magento instance for you.
Hi @AndresInSpace, here is your Magento Instance: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering Admin access: https://45243a10eb203fde7e8eedf63293cfe1.instances-prod.magento-community.engineering/admin_5628 Login: b9724f7c Password: 835839f0d1f0
Hi @AndresInSpace, here is your Magento Instance: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering Admin access: https://0a05fa17a7983d4b098002b067de8a5d.instances-prod.magento-community.engineering/admin_d3bb Login: 40e0096b Password: 7f4a06ea70f6
@magento give me 2.4.7 instance with extensions adyen/module-payment:9.5.3
Hi everyone, any ideas of when this will be merged? We've been suffering lately due to this with the page builder module and generally mixins not always loading preventing saving cms blocks and pages.
Applying this patch temporarily seems to solve the issues.
@Den4ik you self-requested a review 2 months ago -- is there anything left to be reviewed to push this fix forward? Thank you.
Hi @AndresInSpace,
Could you please recheck this PR and resolve the merge conflicts when you have a moment? It's highly anticipated and would be a valuable addition to the community!
Thank you for your efforts :)
@Den4ik Hi, I updated instead of @AndresInSpace and fixed the conflicts in a fork: https://github.com/magento/magento2/compare/2.4-develop...medigeek:magento2:pr-39097-rebased-fixed-conflicts-jan-2025 https://github.com/medigeek/magento2/tree/pr-39097-rebased-fixed-conflicts-jan-2025
most stuff in require.js have already been fixed previously, but the other files weren't. hope it helps.
Hi @medigeek
Thanks for update.
Could you please merge your branch into AndresInSpace:requirejsfix to proceed with current PR.