Skip to content

Magnifier.js syntax error #36200

New issue
New issue
Closed
#39321
Closed
Magnifier.js syntax error#36200
#39321
Assignees
hostep
Labels
Area: FrameworkComponent: CatalogIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P3May be fixed according to the position in the backlog.Progress: PR in progressProgress: doneReproduced on 2.4.xThe issue has been reproduced on latest 2.4-develop branchTriage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it
@vandijkstef

Description

@vandijkstef
vandijkstef
opened on Sep 26, 2022 · edited by engcom-Hotel

Preconditions and environment

  • Mage 2.4.4

Steps to reproduce

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

image

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”.

Activity

m2-assistant

m2-assistant commented on Sep 26, 2022

@m2-assistant
m2-assistantbot
on Sep 26, 2022

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.

⚠️ 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-community-project
added
Issue: ready for confirmation
on Sep 26, 2022
engcom-Delta
added
Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it
on Sep 26, 2022
engcom-Delta
self-assigned this
on Sep 26, 2022
m2-assistant

m2-assistant commented on Sep 26, 2022

@m2-assistant
m2-assistantbot
on Sep 26, 2022

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).

    DetailsIf 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

engcom-Delta commented on Sep 26, 2022

@engcom-Delta
engcom-Delta
on Sep 26, 2022
Contributor

Hi @vandijkstef,

Thank you for reporting and collaboration,

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

Thanks.

engcom-Delta
added
Issue: needs updateAdditional information is require, waiting for response
on Sep 26, 2022
m2-community-project
removed
Issue: ready for confirmation
on Sep 26, 2022
vandijkstef

vandijkstef commented on Oct 4, 2022

@vandijkstef
vandijkstef
on Oct 4, 2022
Author

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-Delta
removed their assignment
on Oct 11, 2022
engcom-Hotel

engcom-Hotel commented on Dec 13, 2022

@engcom-Hotel
engcom-Hotel
on Dec 13, 2022
Contributor

@magento give me 2.4-develop instance

magento-deployment-service

magento-deployment-service commented on Dec 13, 2022

@magento-deployment-service
magento-deployment-servicebot
on Dec 13, 2022

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

engcom-Hotel
self-assigned this
on Dec 13, 2022
m2-assistant

m2-assistant commented on Dec 13, 2022

@m2-assistant
m2-assistantbot
on Dec 13, 2022

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).

    DetailsIf 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

magento-deployment-service commented on Dec 13, 2022

@magento-deployment-service
magento-deployment-servicebot
on Dec 13, 2022

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

31 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Area: FrameworkComponent: CatalogIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P3May be fixed according to the position in the backlog.Progress: PR in progressProgress: doneReproduced on 2.4.xThe issue has been reproduced on latest 2.4-develop branchTriage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @hostep@vandijkstef@engcom-Bravo@engcom-Delta@engcom-Hotel

    Issue actions
      Magnifier.js syntax error · Issue #36200 · magento/magento2