Fix customer data and sections configuration race condition

[krzksz]

This pull request aims to fix all issues connected with the fact that the configuration of customer data and its sections is currently being passed through x-magento-init script block.

Description (*)

The majority of information is already provided within #17125 and #23099. Because of the fact that both customer-data.js and section-config.js modules rely on x-magento-init script to initialize them with configuration there are situations in which they can be required and evaluated by other modules before that actually happens which results in multiple errors e.g.:

• TypeError: sections is undefined
• Uncaught TypeError: Cannot set property 'sectionLoadUrl' of undefined
• TypeError: Cannot read property '*' of undefined

My initial idea was to pass mentioned configuration simply via a global variable but then I remembered that RequireJS actually has a build-in mechanism for that https://requirejs.org/docs/api.html#config-moduleconfig which I took advantage of. This pull request also makes changes within #23099 obsolete.

Fixed Issues (if relevant)

1. x-magento-init initialisation not bound to happen in the right order. #17125: x-magento-init initialisation not bound to happen in the right order.
2. Magento 2.2.3 TypeErrors Cannot read property 'quoteData' / 'storecode' / 'sectionLoadUrl' of undefined  #14412 Magento 2.2.3 TypeErrors Cannot read property 'quoteData' / 'storecode' / 'sectionLoadUrl' of undefined
3. Magento 2.1.11 CE ADD TO CART is stuck on ADDING ... #13403 Magento 2.1.11 CE ADD TO CART is stuck on ADDING
4. cannot read property 'section loadurl' of undefined in all pages - Custom Theme #8628 cannot read property 'section loadurl' of undefined in all pages - Custom Theme

Manual testing scenarios (*)

1. Reload the shop.
2. Trigger customer data or sections data module at the very beginning for example by running the following code:

require(['Magento_Customer/js/section-config'], function(sectionsConfig) {
    console.log(sectionsConfig.getAffectedSections('test'));
});

3. TypeError: sections is undefined is being thrown without this changes.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @krzksz. Thank you for your contribution
Here is some useful tips 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.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[krzksz]

@magento give me 2.3-develop instance

[magento-engcom-team]

Hi @krzksz. Thank you for your request. I'm working on Magento 2.3-develop instance for you

[magento-engcom-team]

Hi @krzksz, here is your Magento instance.
Admin access: https://i-24847-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

[krzksz]

@magento give me test instance

[magento-engcom-team]

Hi @krzksz. Thank you for your request. I'm working on Magento instance for you

[magento-engcom-team]

Hi @krzksz, here is your new Magento instance.
Admin access: https://pr-24847.instances.magento-community.engineering/admin
Login: admin Password: 123123q

[sidolov]

@DrewML could you please take a look at the changes?

[krzksz]

FYI: it seems like some sections are not invalidated now property hence failing tests, I'm looking into it. pt., 4 paź 2019, 22:35 użytkownik Stanislav Idolov <notifications@github.com> napisał:

…

@DrewML <https://github.com/DrewML> could you please take a look on the changes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24847?email_source=notifications&email_token=AAGKOBONSAJNXKEADBDBMD3QM6SIXA5CNFSM4I5MNBP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAM2A6Y#issuecomment-538550395>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGKOBMZKXLP3GFI5T6ZXB3QM6SIXANCNFSM4I5MNBPQ> .

[krzksz]

Ok, it seems that everything is back to normal. Currently the only errors that are left are:

• Usage of $this in the template to access helper.
• Inline JavaScript in script tags.

Please @DrewML and @paliarush what I should do with them.

[DrewML]

I'm a bit backed up with work at the moment, but tagging in @vzabaznov to try and get a review done. He's done a massive amount of investigation into customer-data problems over the last few weeks.

[vzabaznov]

Hi @krzksz, thank you for PR.
Dont think using require.config is what we need here, because this will not fix this problem globally.
I believe that better approach here is to use our uiRegistry - https://devdocs.magento.com/guides/v2.3/ui_comp_guide/concepts/ui_comp_uiregistry.html. Can you take a look at it?

[DrewML]

Have we checked yet if using require.config is going to work with mixins? In other words, if I want to mixin section-config.js, will module.config() in that mixed-in module be able to obtain the needed data?

[krzksz]

@ya

Hi @krzksz, thank you for PR.
Dont think using require.config is what we need here, because this will not fix this problem globally.
I believe that better approach here is to use our uiRegistry - https://devdocs.magento.com/guides/v2.3/ui_comp_guide/concepts/ui_comp_uiregistry.html. Can you take a look at it?

Hmm, I'm not sure what the actual approach should be, but I'm guessing we should try to provide the information via registry.async as it waits for the component to be initialized. I hope that's what you mean and so I'll try to investigate but I don't really know if it is the best option and what you mean by this will not fix this problem globally. Can you elaborate a bit?

Have we checked yet if using require.config is going to work with mixins? In other words, if I want to mixin section-config.js, will module.config() in that mixed-in module be able to obtain the needed data?

Good point, will check.

[krzksz]

@DrewML

As suspected module.config() inside mixin returns the configuration for that said mixin which means that it would not be possible to customize section or customer data configuration frontend-only without modifying the template. Is it really a problem? Maybe adding another method to update the configuration would be enough?

@vzabaznov I tried to figure it out but I still don't know how uiRegistry could be applied here 🤔

[m2-assistant]

Hi @krzksz, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[krzksz]

@magento give me 2.3-develop instance

[magento-engcom-team]

Hi @krzksz. Thank you for your request. I'm working on Magento 2.3-develop instance for you

[magento-engcom-team]

Hi @krzksz, here is your Magento instance.
Admin access: https://i-24847-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

[krzksz]

I am closing this PR because its changes would do more damage then help. I decided to prepare a separate one where I'm covering both section-config and customer-data with unit tests and make sure they do not throw any errors before they are initialized.

[m2-assistant]

Hi @krzksz, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.