Skip to content
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

Fix: product-sorter - scope-selector readiness detection #1375

Conversation

allanpaiste
Copy link

@allanpaiste allanpaiste commented Apr 11, 2019

Random crash on Catalog >> Category page in admin.

The issue is much easier to replicate when you enable xDebug. Overall the problem is caused by a race-condition of different ui-components, which means that it's occurring randomly and might take some time to repeat.

Uncaught SyntaxError: Unexpected end of JSON input (product-sorter.js.125)

Pre-requisites

  1. Use latest M2 2.2.X release (2.2.8 at the time of writing).
  2. Enable xDebug

Steps

  1. Log into admin
  2. Navigate to Catalog >> Categories
  3. Select any category (root category is also fine)
  4. Reload the Category Page repeatedly

Expected

  1. Category page is fully loaded and categories are shown

Actual

  1. Category page is not loaded fully, loading indicator is shown, js error in console

Symptoms

The sorted_products initiates sometimes (because require.js is async and initiates components somewhat randomly when they're on same level) before the ui.checkbox for use_store_positions which means that switchScope triggers when it's default value is being set.

This is not intended behaviour and the product sorting control actually switch to disabled state even when in store_id=0 scope.

Discussion

Luckily for us, the ui-components listens VS imports has been built so that the processing of imports mapping triggers after value change listens (I presume that the previous code in this specific JS file already was trying to fix the same issue that this PR is re-visiting); This means that while the initial value is being set (when the components that we're listening) is not yet fully initiated, the import target is not even properly set yet.

Therefore the scopeSwitcher (import target) is not yet set (undefined), and will be set AFTER the source element has properly initiated.

Solution

This could be seen as a core M2 bug as one could argue that listeners should not be notified when the thing they're listening to is not yet properly set up.

But in the context of product-sorter: we can just ignore calls to switchScope until the imported value of scopeSwitcher has actually been imported (which means that it ends up being either 1 or 0, but definitely not undefined).

NOTE: even if we see this as core bug, you'd still want this module to provide a solution that works for all M2 releases.

@allanpaiste allanpaiste changed the title Improve logic around detecting scope selector state Fix: product-sorter - improve logic around detecting scope selector readiness Apr 11, 2019
@allanpaiste allanpaiste changed the title Fix: product-sorter - improve logic around detecting scope selector readiness Fix: product-sorter - improve scope-selector readiness detector (product-sorter) Apr 11, 2019
@allanpaiste allanpaiste changed the title Fix: product-sorter - improve scope-selector readiness detector (product-sorter) Fix: product-sorter - improve scope-selector readiness detector Apr 11, 2019
@allanpaiste allanpaiste changed the title Fix: product-sorter - improve scope-selector readiness detector Fix: product-sorter - scope-selector readiness detection Apr 11, 2019
@romainruaud
Copy link
Collaborator

Hello @allanpaiste

First of all : wow, thanks ! Such a deep dive into Ui Components logics, clear PR, and solution sounds good to me so far.

We'll probably merge this one, can you just rebase it on 2.6.x branch of ElasticSuite and point the PR accordingly ?

More personal : I see that you are working for Vaimo, which a pretty cool company that we do respect much. We are interested in meeting people responsible for partnership in Vaimo, to discuss about ElasticSuite future & roadmap, maybe show you some demo of cool stuff we have under the hood, and see how we could leverage our synergy. Would you mind sending me an email at romain.ruaud@smile.fr to point me to the right persons by your side ?

Best regards

@romainruaud
Copy link
Collaborator

Replaces by #1385 and #1386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants