Magnifier.js syntax error #36200

vandijkstef · 2022-09-26T10:05:54Z

Preconditions and environment

Mage 2.4.4

Steps to reproduce

Open the console on the Product Detail page and define const data={} in the console.
Try to navigate the image gallery and get the below error:

3. But if replace `;` to `,` from `lib/web/magnifier/magnifier.js`, then the issue is resolved.

./lib/web/magnifier/magnifier.js 26-30 is causing an error depending on contexts

           largeWrapper = options.largeWrapper || '.magnifier-preview',
            $magnifierPreview = $(largeWrapper);

        curThumb = null,
        magnifierOptions = {

It errors out later in the script, saying I cannot redefine const data. This is because the list of variables starting at curThumb doesn't have specified what kind of var it is, thus living in global space.

So a fix would either be

// Comma instead of ;
$magnifierPreview = $(largeWrapper),
curThumb = null,

Or properly initiate the variables

$magnifierPreview = $(largeWrapper);

var curThumb = null,

Expected result

No JS errors

Actual result

JS breaks, product detail page image gallery is not functional anymore.

Additional information

I wouldn't know which solution is preferred which is why I just drop this as an issue with possible solutions provided. Additionally, it would be great to have proper JS linting set up, that doesn't allow global variable declaration like this.

As far as I can see, this has been an issue/unchanged for a few Magento version, it just got triggered by another module/js taking 'control' of the 'data' var-label.

Release note

No response

Triage and priority

Severity: S0 - Affects critical data or functionality and leaves users without workaround.
Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

The text was updated successfully, but these errors were encountered:

m2-assistant · 2022-09-26T10:05:56Z

Hi @vandijkstef. Thank you for your report.
To speed up processing of this issue, make sure that you provided the following information:

Summary of the issue
Information on your environment
Steps to reproduce
Expected and actual results

Make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, review the Magento Contributor Assistant documentation.

Add a comment to assign the issue: @magento I am working on this

To learn more about issue processing workflow, refer to the Code Contributions.

Join Magento Community Engineering Slack and ask your questions in #github channel.

⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

m2-assistant · 2022-09-26T15:15:26Z

Hi @engcom-Delta. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
5. Add label Issue: Confirmed once verification is complete.
6. Make sure that automatic system confirms that report has been added to the backlog.

engcom-Delta · 2022-09-26T15:32:14Z

Hi @vandijkstef,

Thank you for reporting and collaboration,

Can you please provide more detail on which context you are getting error.

Thanks.

vandijkstef · 2022-10-04T08:51:21Z

Hi @engcom-Delta,

Sorry I missed your question for a bit. Basically I have another js-script running that is trying to set a variable const data. The variable initialisation of the magnifier script is interupted by the ; on the line containing $magnifierPreview = $(largeWrapper); making every variable after it global, as it's not prefixed by var|let|const anymore.

Here's an interesting thread about the subject

engcom-Hotel · 2022-12-13T07:08:37Z

@magento give me 2.4-develop instance

magento-deployment-service · 2022-12-13T07:08:39Z

Hi @engcom-Hotel. Thank you for your request. I'm working on Magento instance for you.

m2-assistant · 2022-12-13T07:08:46Z

Hi @engcom-Hotel. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
5. Add label Issue: Confirmed once verification is complete.
6. Make sure that automatic system confirms that report has been added to the backlog.

magento-deployment-service · 2022-12-13T07:12:18Z

Hi @engcom-Hotel, here is your Magento Instance: https://41efee41a835c031623b15b144cde72f.instances.magento-community.engineering
Admin access: https://41efee41a835c031623b15b144cde72f.instances.magento-community.engineering/admin_3918
Login: 96e5af72
Password: 100afafa9b1d

engcom-Hotel · 2022-12-13T07:35:36Z

Hello @vandijkstef,

Thanks for the reporting and collaboration!

We have tried to reproduce the issue on the above Magento instance and found that curThumb variable is in the Global scope, please refer the screenshot:

But still, the Image gallery is working fine for us. Can you please let us know any additional steps which we need to follow in order to reproduce the issue?

Thanks

vandijkstef · 2022-12-13T09:58:00Z

Basically I have another js-script running that is trying to set a variable const data.

curThumb is just the start of this list of global variables. Having global variables is just asking for unintended side-effects and incompatibility issues. Please stop using global variables. Any module (accidently) using one of these variable names will have issues.

It's highly likely to me that $magnifierPreview = $(largeWrapper); used to end with nothing, and a linter added ; to that line

engcom-Hotel · 2022-12-14T08:21:49Z

Hello @vandijkstef,

Thanks for the reply!

According to your comment, we have tried to reproduce the issue by following the below steps:

Open the console on the Product Detail page and define const data={} in the console.
Try to navigate the image gallery and get the below error:

3. But if replace `;` to `,` from `lib/web/magnifier/magnifier.js`, then the issue is resolved.

But I think this is not the proper way to reproduce the issue. So can you please help us in the same?

Thanks

vandijkstef · 2022-12-14T13:52:58Z

This is the proper way to reproduce this issue. This is the proper solution. It's all about JS coding standards not being applied here. Any of the following variables will not be usable by any other module

curThumb
magnifierOptions
pos
gId
status
curIdx
curLens
curLarge
lensbg
gZoom
gZoomMin
gZoomMax
gMode
gEventTypemagnifierOptions.eventType,
data
inBounds
isOverThumb
rate
paddingX
paddingY
enabled
showWrapper

If I look the the old version of the file (it's a few years back) you can see what was originally in there, and that a ; was inserted around here likely without the intend to do so, but a linter accidently inserting the ;

engcom-Hotel · 2022-12-15T09:41:30Z

Hello @vandijkstef,

Thanks for the update!

We are confirming this issue and updating the main description with the steps mentioned here #36200 (comment)

Thanks

github-jira-sync-bot · 2022-12-15T09:44:49Z

✅ Jira issue https://jira.corp.adobe.com/browse/AC-7440 is successfully created for this GitHub issue.

m2-assistant · 2022-12-15T09:44:52Z

✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

vandijkstef · 2024-06-25T07:54:25Z

This is still an issue in Mage 2.4.7 - We're talking about replacing a ; with a , after faulty auto-formatted js code

m2-community-project bot added the Issue: ready for confirmation label Sep 26, 2022

engcom-Delta added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Sep 26, 2022

engcom-Delta self-assigned this Sep 26, 2022

engcom-Delta added the Issue: needs update Additional information is require, waiting for response label Sep 26, 2022

m2-community-project bot removed the Issue: ready for confirmation label Sep 26, 2022

engcom-Delta removed their assignment Oct 11, 2022

engcom-Hotel self-assigned this Dec 13, 2022

engcom-Hotel added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Priority: P3 May be fixed according to the position in the backlog. Area: Framework labels Dec 15, 2022

m2-community-project bot added the Progress: dev in progress label Dec 15, 2022

m2-assistant bot unassigned engcom-Hotel Dec 15, 2022

m2-community-project bot added Progress: ready for dev and removed Progress: dev in progress labels Dec 15, 2022

engcom-Hotel removed the Issue: needs update Additional information is require, waiting for response label Dec 15, 2022

engcom-Hotel added this to Low Priority Backlog Aug 19, 2024

engcom-Hotel moved this to Ready for Development in Low Priority Backlog Aug 19, 2024

hostep linked a pull request Nov 3, 2024 that will close this issue

Move bunch of javascript variables from global scope to local scope in magnifier.js file #39321

Open

5 tasks

m2-community-project bot assigned hostep Nov 3, 2024

m2-community-project bot added Progress: PR in progress and removed Progress: ready for dev labels Nov 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Magnifier.js syntax error #36200

Magnifier.js syntax error #36200

vandijkstef commented Sep 26, 2022 •

edited by engcom-Hotel

Loading

m2-assistant bot commented Sep 26, 2022

m2-assistant bot commented Sep 26, 2022

engcom-Delta commented Sep 26, 2022

vandijkstef commented Oct 4, 2022

engcom-Hotel commented Dec 13, 2022

magento-deployment-service bot commented Dec 13, 2022

m2-assistant bot commented Dec 13, 2022

magento-deployment-service bot commented Dec 13, 2022

engcom-Hotel commented Dec 13, 2022

vandijkstef commented Dec 13, 2022

engcom-Hotel commented Dec 14, 2022

vandijkstef commented Dec 14, 2022 •

edited

Loading

engcom-Hotel commented Dec 15, 2022

github-jira-sync-bot commented Dec 15, 2022

m2-assistant bot commented Dec 15, 2022

vandijkstef commented Jun 25, 2024

Magnifier.js syntax error #36200

Magnifier.js syntax error #36200

Comments

vandijkstef commented Sep 26, 2022 • edited by engcom-Hotel Loading

Preconditions and environment

Steps to reproduce

Expected result

Actual result

Additional information

Release note

Triage and priority

m2-assistant bot commented Sep 26, 2022

m2-assistant bot commented Sep 26, 2022

engcom-Delta commented Sep 26, 2022

vandijkstef commented Oct 4, 2022

engcom-Hotel commented Dec 13, 2022

magento-deployment-service bot commented Dec 13, 2022

m2-assistant bot commented Dec 13, 2022

magento-deployment-service bot commented Dec 13, 2022

engcom-Hotel commented Dec 13, 2022

vandijkstef commented Dec 13, 2022

engcom-Hotel commented Dec 14, 2022

vandijkstef commented Dec 14, 2022 • edited Loading

engcom-Hotel commented Dec 15, 2022

github-jira-sync-bot commented Dec 15, 2022

m2-assistant bot commented Dec 15, 2022

vandijkstef commented Jun 25, 2024

vandijkstef commented Sep 26, 2022 •

edited by engcom-Hotel

Loading

vandijkstef commented Dec 14, 2022 •

edited

Loading