Fix issue disabled modules still include less

[mrtuvn]

Description (*)

Retarget my old PR to new branch

This PR bring the patch for fix issue modules disabled still include less files. It's should not included in output final style files. Previously styles still included in output final css styles

Use case 1: Disable module example Company_CustomCatalog in app/etc/config.php. But assume accidental add module styles in theme folder in
app/design/frontend/theme/theme_name/Company_CustomCatalog/web/css/source/_module.less
app/code/Company/CustomModule/web/css/source/_module.less
Actually after run deploy output css magento still included Company_CustomCatalog styles in file styles-m.css in pub folder

Previously magento collect files get all files less from web dir theme follow dependency but not check files belong modules enabled or not. So some files from inactive modules (file _module.less) from theme folders still be included. That's is what we don't want in real use-case world

After patch files get correctly. If module has disabled both styles from web module area and theme area will not included

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes Static content is deploying for disabled modules #24666 Static content is deploying for disabled modules
   Keep continue works by @Echron after long inactivity

Manual testing scenarios (*)

1. Magento 2.3.4+
2. blank or luma theme
3. Disable a couple module use less styles: Ex: Magento_Review, Magento_LoginAsCustomerFrontendUi
4. Run deploy static content and clear cache

Questions or comments

For this pr seem will take quite long period for acceptance and delivery to 2.5 branch. In the meantime we can use this patch
https://github.com/vasilii-b/magento2-frontend-improvements/blob/master/patches/composer/magento-framework/import-styles-for-enabled-modules-only.patch. for issue
Use this with your own risk! (not official patch from magento) ( but it's works)

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @mrtuvn. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.5-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests 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 Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

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

[ihor-sviziev]

@magento run all tests

[mrtuvn]

Old PR link for anyone can reference #27888

[ihor-sviziev]

Just moved all labels from #27888

[ihor-sviziev]

@magento run Functional Tests B2B, Functional Tests CE

[ihor-sviziev]

✔ Approved.

Failing tests looks not related to changes form this PR.

[ihor-sviziev]

@magento run Functional Tests B2B

[mrtuvn]

Rebased 2.5-develop upstream

[mrtuvn]

@magento run all tests

[mrtuvn]

Hi guys any update on this ?

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8610 has been created to process this Pull Request

[hostep]

Is there any progress here?

Would be nice if this could get included in Magento 2.5 since it could drastically reduce the filesize of generated css in Magento

[mrtuvn]

I think this one may take a long time delivery to codebase. Quite long response from internal

Meanwhile we have 2 options workaround here both affect/modify to framework area

1. Follow this patch created by @vasilii-b https://github.com/vasilii-b/magento2-frontend-improvements/blob/master/patches/composer/magento-framework/import-styles-for-enabled-modules-only.patch
   simple workaround seems to work correct and less files change(1 file)
2. Patch approach of this PR (more files changed) (2 files)

I'm sure may still have room for a better fix

Should we update for this priority to P2 ? Without workarounds i think this behavior will affect to all merchants stores
cc: @sivaschenko @gabrieldagama

[ihor-sviziev]

Hi @mrtuvn,
I'm closing this PR as we have another one #32922 that looks way cleaner, faster, and easier to add test coverage to it.

Thank you so much for your contribution!

[m2-assistant]

Hi @mrtuvn, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[hostep]

@ihor-sviziev : during Magento Live in 2019 in Amsterdam, me and some buddies discussed this during the contribution day with core Magento devs and they told us that it was better to implement it this way using decorators in xml files.

See #25183 (comment)

Just FYI

Frankly I don't mind how it gets implemented, as long as the end result is the same.

[ihor-sviziev]

@hostep, yeah, remember that, but still, it's better to use the KISS principle here. It will be way clearer how it works, so easier for maintaining and less potential to have some regression there.

[mrtuvn]

This one still better implements imho. But maybe should have more additional such as this comment #32922 (comment)