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.

Sign up for GitHub

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

Jump to bottom

JS files located outside the web/js directory #16302

Closed
BenSpace48 opened this issue Jun 21, 2018 · 11 comments
Closed

JS files located outside the web/js directory #16302

BenSpace48 opened this issue Jun 21, 2018 · 11 comments
Assignees
@hitesh-wagento
Labels
Component: Framework/Code Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@BenSpace48
Copy link
Contributor

Summary

Some JS files are direct children of web rather than web/js. This does not follow instructions from the dev docs thus is confusing.

Place the custom component source file in one of the following locations:
Your theme JS files: /web/js or /_/web/js
Your module view JS files: <module_dir>/view/frontend/web/js

There are more instances of this:

  • vendor/magento/module-payment/view/frontend/web/transparent.js
  • vendor/magento/module-payment/view/frontend/web/cc-type.js
  • vendor/magento/module-search/view/frontend/web/form-mini.js
  • vendor/magento/module-sales/view/frontend/web/gift-message.js

There are more but I'm pushed for time at the minute.

Preconditions

  1. Magento 2.2 - This is irrelevant as it appears in all versions

Steps to reproduce

  1. Go to https://github.com/magento/magento2/tree/2.2-develop/app/code/Magento/Captcha/view/frontend/web and look where captcha.js and onepage.js is located.

Expected result

  1. JS files to be inside web/js as per the dev docs.

Actual result

  1. JS files are direct children of web, this is inconsistent with the dev docs.

This is confusing as the dev docs instructs developers to use certain locations yet the core does not follow this, either the files should be relocated to be inside web/js or the dev docs should be updated. Unless there is a valid reason why the files are outside web/js?
@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jun 21, 2018
@magento-engcom-team
Copy link
Contributor

magento-engcom-team commented Jun 21, 2018 •
edited by BenSpace48
Loading

Hi @BenSpace48. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

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

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting with 2.2.0+) or develop branches (2.2-develop +).
For more details, please, review the Magento Contributor Assistant documentation

@ghost ghost self-assigned this Jul 4, 2018
@ghost ghost added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Jul 4, 2018
@ghost
Copy link

ghost commented Jul 4, 2018

@BenSpace48, thank you for your report.
We've acknowledged the issue and added to our backlog.

@hitesh-wagento
Copy link
Contributor

hitesh-wagento commented Jul 5, 2018 •
edited
Loading

Hi @BenSpace48

Do you need below files in web/js folder and captcha.js and onepage.js ?

  • vendor/magento/module-payment/view/frontend/web/transparent.js
  • vendor/magento/module-payment/view/frontend/web/cc-type.js
  • vendor/magento/module-search/view/frontend/web/form-mini.js
  • vendor/magento/module-sales/view/frontend/web/gift-message.js

@BenSpace48
Copy link
Contributor Author

Hi @hitesh-wagento

Yeah for consistency with the dev docs and other files, and general best practices.

I think there are more files than the ones I've listed, they were just the first few I discovered whilst looking into it.

@hitesh-wagento
Copy link
Contributor

Ok Thanks @BenSpace48

I am working on this PR

@hitesh-wagento hitesh-wagento self-assigned this Jul 5, 2018
This was referenced Jul 6, 2018
Merged
Merged
@ghost ghost removed their assignment Jul 12, 2018
@magento-engcom-team
Copy link
Contributor

Hi @BenSpace48. Thank you for your report.
The issue has been fixed in #16582 by @hitesh-wagento in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.7 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Aug 7, 2018
@magento magento deleted a comment from magento-engcom-team Aug 23, 2018
@BenSpace48
Copy link
Contributor Author

@magento-engcom-team give me 2.2.5 instance

@magento-engcom-team
Copy link
Contributor

Hi @BenSpace48. Thank you for your request. I'm working on Magento 2.2.5 instance for you

@magento-engcom-team
Copy link
Contributor

Hi @BenSpace48, here is your Magento instance.
Admin access: https://i-16302-2-2-5.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@ishakhsuvarov ishakhsuvarov added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Sep 12, 2018
@ishakhsuvarov
Copy link
Contributor

Hi @BenSpace48. Thank you for your report.
The issue has been fixed in #16708 by @hitesh-wagento in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

rbayet added a commit to rbayet/elasticsuite that referenced this issue Nov 19, 2018
@rbayet
Fixes Smile-SA#1186 - support web/js files relocation
64ee4c2 
To be introduced in 2.3.x and 2.2.7
With expected BC break in 2.2.x because magento/magento2#16582 does not seem to cover this particular case
See issue of origin magento/magento2#16302
This was referenced Nov 19, 2018
Merged
Closed
@rbayet rbayet mentioned this issue Dec 5, 2018
Merged
4 tasks
@magento-engcom-team
Copy link
Contributor

Hi @BenSpace48. Thank you for your report.
The issue has been fixed in #19583 by @rbayet in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.1 release.

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

@hitesh-wagento hitesh-wagento

Labels
Component: Framework/Code Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ishakhsuvarov @BenSpace48 @hitesh-wagento @magento-engcom-team