fix customer data race condition when bundling is enabled

[davidverholen]

Description (*)

when using advanced bundling there is a race condition leading to an error where options.sectionLoadUrl is undefined when customer data tries to retrieve sections from the server.

This has already lead to several issues with mostly different context but the same base problem: the options object is initialized as x-magento-init script in a phtml file which is not always executed fast enough (for example if you use the advanced bundling).

A fix for that will also be mandatory for tools that automate an advanced bundling mechanism (/cc @DrewML )

some related tickets that have been closed:
https://github.com/magento/magento2/issues?q=is%3Aissue+sectionLoadUrl+is%3Aclosed

related issue in m2-devtools project:
magento/m2-devtools#56

Manual testing scenarios (*)

1. enable production mode
2. implement advanced bundling (maybe it also happens with magento default bundling)
   -> For example by using: https://github.com/magento/m2-devtools

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 @davidverholen. 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 Assistant documentation

[DrewML]

Thank you for taking the initiative on this at the Hackathon! Will be reviewing as soon as I'm back from travel (possibly sooner)

[DrewML]

Sorry for the delays, and thanks for doing the hard work!

For future readers, want to make sure we're documenting the exact problem we're trying to solve in this PR.

Currently, customer-data.js initializes a variable (options) to undefined in the top-most lexical scope in the module.

An exported property with the name Magento_Customer/js/customer-data is later invoked by a mage-init, which passes in an object that is then assigned to options in the closure.

Unfortunately, there are other APIs exposed from customer-data.js that rely on options being populated with data. If any of that code runs before the mage-init widget, then we throw an exception because options is undefined, but we access properties on it.

So basically, this problem is temporal coupling.

[DrewML]

re: the fix, I assume you (and most others) know this section of the code better than me.

Can you help me understand what the situations are will this code will break down? Basically, I assume we didn't hard code the sectionLoadUrl for some reason, but no idea why. Now that we hardcode it, I want to understand what scenarios could break for folks.

[henk-hypershop]

Thank you for taking the initiative on this at the Hackathon! Will be reviewing as soon as I'm back from travel (possibly sooner)

Awesome! @davidverholen fix worked like a charm. Thanks!

[davidverholen]

@DrewML the url is static ( / defined in module code) so it should not be a problem to hardcode it (it is already hardcoded like this in php view code / in the block anyway)

When a customer section is updated (for example adding a product to the cart) a function in the customer-data.js is called to reload the respective customer section from the backend. This call can also happen directly after a site reload (e.g. when not using an ajax call to add a product to cart).

The options object is initialized in the body by the mage-init script while the ajax call is executed at some point when all dependencies are loaded (and I think you mentioned that it even is blocked until all js is loaded on the page).

When the js is not bundled, it takes a huge amount of time to get to this point so the options object is mostly already initialized.

When the js is bundled and minified, the customer section update is triggered before the initialization of the options object.

The function then crashes with an error because it relies on options.sectionLoadUrl to execute the call

[DrewML]

There is probably a larger fix we could make here to just eliminate the race, BUT this at least gets us from a place where things fail for shoppers to a place where things should mostly succeed, and without breaking changes, so it's a 👍 from me.

Thanks again

[magento-engcom-team]

Hi @kalpmehta, thank you for the review.
ENGCOM-5457 has been created to process this Pull Request
✳️ @kalpmehta, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Delta]

✔️ QA passed

[m2-assistant]

Hi @davidverholen, 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.