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 JS errors when Product is not found #370

Merged
merged 5 commits into from
Aug 14, 2020

Conversation

dhardtke
Copy link
Contributor

@dhardtke dhardtke commented Aug 10, 2020

Description

  1. When the galleryRoot element is not present in the DOM, the corresponding component attempts to use an empty object for constructing the list of gallery items. Unfortunately, this object is passed to JSON.parse() which does not accept objects and the code crashes with a SyntaxError.
  2. When the SKU element is not present, accessing its innerHTML fails
  1. Instead of parsing an object when the galleryRoot is unpresent, the code now simply avoids parsing completely and instead constructs an object to be used as fallback directly.
  2. Before accessing the SKU element's innerHTML it is checked that the element exists

Related Issue

Fixes #281.
Fixes #369.

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

JSON.parse() only accepts strings. When the galleryRoot was
not present we passed an object which led to a SyntaxError.
@dhardtke dhardtke changed the title Fix JSON.parse error when galleryRoot is not present in DOM Fix JS errors when Product is not found Aug 10, 2020
@herzog31 herzog31 added the bug Something isn't working label Aug 10, 2020
@herzog31
Copy link
Member

lgtm 👍

Could you please add a test case each for your fixes? You can find related test cases in https://github.com/adobe/aem-core-cif-components/tree/master/ui.apps/test/components/commerce/product.

@dhardtke
Copy link
Contributor Author

lgtm +1

Could you please add a test case each for your fixes? You can find related test cases in https://github.com/adobe/aem-core-cif-components/tree/master/ui.apps/test/components/commerce/product.

I added a test for the change in product.js but I'm not sure on how to test the change in the Gallery instantiation: The function there is never called in the tests. We'd probably have to move the onDocumentReady function out of the IIFE and call it manually to test it doesn't throw.

@herzog31 herzog31 merged commit 58a61e0 into adobe:master Aug 14, 2020
@herzog31
Copy link
Member

@dhardtke Thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working verified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JavaScript errors on Product pages if Product can't be found JSON Parse error for gallery.js
6 participants