Add logic check only import css from enabled modules

[mrtuvn]

Description (*)

This PR based on workarounds @vasilii-b for resolve this remain issue. Default magento import all css from all modules in (app/etc/config.php) no matter it was enabled or not.
See file less in var/view_preprocessed/pub/static/frontend/Vendor/theme/locale/css
For normal site with small customise module css size is not problem but imagine store have a lot of modules and each module have custom styles. Css output size will be huge and delay browser parse css

when see file styles-m.less or styles-l.less, you can see this line

//@magento_import 'source/_module.less'; // Theme modules
//@magento_import 'source/_widgets.less'; // Theme widgets

It's not actually comment code. This is syntax by magento allow import external modules and widgets styles across in all modules and themes

Current exists problem
If we disabled module but we add styles inside theme -> these styles in theme also added to final output instead expect not available in final css.

Benefits/Expects
assume If module disabled so that styles listed below should not be include in output styles

@import '../Vendor_Module/css/source/_extend.less'; //Should not include
@import '../Vendor_Module/css/source/_module.less'; //Should not include
@import '../Vendor_Module/css/source/_widgets.less'; //Should not include

disabled module is deprecated but some case if we need or know exactly problem come from some module. We can disabled it and also make sure styles come from theme of that module will not parse at final output css.
The benefits => reduce size outputs css file

Some downside/edge-cases

Case 1 (Rarely): If we add styles of disabled module to enabled module. We can't exclude it. I'm not sure in real-world we may got this case (look like it's related with manager code/quality)
Example
Magento_Review -> disabled

in theme luma theme-frontend-luma/Magento_Catalog/web/css/source/_extend.less

@import 'Magento_Review::css/custom.less';

theme-frontend-luma/Magento_Review/web/css/custom.less <= styles in here still loaded. Only styles from files, _module.less, widget, _extend will able wipe out

Known limitations
Some variables added/defined in module less. But some other files such as (_extends.less) reference/use/call to it. One case will false if we disabled module
Solutions: Need refactor files to loose tight-coupling

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes Static content is deploying for disabled modules #24666

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, PageBuilder (note backend function will not work, but frontend styles will expected not available)
4. Add inside to app/etc/env.php or app/etc/config.php

'static_content_only_enabled_modules' => true,

5. Run setup upgrade and deploy static content and clear cache
6. Find class belong modules disabled should not available at final css output files(styles-m.css,styles-l.css)

Questions or comments

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)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @mrtuvn. Thank you for your contribution
Here are 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.4-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

[mrtuvn]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[mrtuvn]

@magento run Unit Tests, Functional Tests EE, Functional Tests CE, Database Compare, Integration Tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[mrtuvn]

@magento run Unit Tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[ihor-sviziev]

@mrtuvn, TBH this solution looks better and much simpler to me. The only concern I still have about it - it’s backward incompatible and might cause issues for some people/themes.
Maybe we can make it configurable and disabled by default? Later on, we'll change it to enabled by default it 2.5

Also, could you cover your changes with unit tests?

[mrtuvn]

issue can reproduce if you add css/less styles in theme area (app/design). You can add file like this to theme

example
app/design/frontend/Magento/blank/Paypal_Braintree/web/css/source/_extend.less
or
app/design/frontend/VendorName/themeName/Paypal_Braintree/web/css/source/_extend.less

In real world case you don't want to modify directly styles in module area

I want to clarify
1.It always (not depending on the flag) does not output styles from the disabled modules. This is not true if you add styles in theme. Current in 2.4-develop branch this only correct if styles add in module scope/area. But in theme area (app/design) no matter you have enable or disabled module (in app/etc/config.php) magento always build css of disabled module to final files(styles-l.css, styles-m.css)

This PR aims to solve that problem. If you disabled module and you added styles before in theme area -> So we don't want that styles (in theme) should be visible at frontend/customers

[engcom-Echo]

@magento run all tests

[engcom-Echo]

✔️ QA Passed

Issue addressed in this PR: Classes belongs to disabled modules should not available at final css output files(styles-m.css,styles-l.css)

Solution given & Expected behaviour: In this PR, “static_content_only_enabled_modules” this flag is introduced. We have to set the flag 'static_content_only_enabled_modules' => 0 or 1 and needs to check the pub\static\frontend\Magento\blank\en_US\styles-m.css file and here are three scenarios with expected output

It always (not depending on the flag) does not output styles from the disabled modules.
With flag set to 1 like 'static_content_only_enabled_modules' => 1 - we should NOT have braintree styles
With flag set to 0 like 'static_content_only_enabled_modules' => 0 - we should HAVE braintree styles
Note: In all the cases, the braintree module is disabled

Now in order to proceed on testing, I have tried to reproduce the issue on latest 2.4-develop
Preconditions:
1 Magento installation

Manual testing scenario:
1 Install any new module (eg- Braintree payment) and do the necessary configurations in the admin side.
2 Create app/design/frontend/Magento/blank/Paypal_Braintree/web/css/source/_extend.less and add custom css
3 Disable the module; and delete the content from var/view_preprocessed/* pub/static/*
4 Run static content deploy: bin/magento setup:static-content:deploy -f
5 Look at the generated file pub/static/frontend/Magento/blank/en_US/css/styles-m.css

Before: ✖️ 

After: ✔️  

Moving it to Extended Testing

[engcom-Echo]

@magento run Functional Tests EE,Functional Tests CE,Functional Tests B2B,WebAPI Tests

[engcom-Echo]

Functional Tests EE,Functional Tests CE,Functional Tests B2B some failure are known failure and other are different on last two run on same commit.

Known Failure
ACQE-3807
ACQE-6450
ACQE-6018
ACQE-5677
ACQE-6438
ACQE-6454
ACQE-6349
ACQE-6438
ACQE-6331

Functional Tests B2B
Run1

Run2

Functional Tests CE
Run1

Run2

Functional Tests EE
Run1

Run2

Web Api Tests failure is known failure

Hence moving it to Merge In Progress

[engcom-Charlie]

@magento run all tests

[hostep]

PR isn't marked as merged (probably because of the additional commit after it got accepted), but seems to be merged in this bigger merge: ada8d1d.

Finally, this has been in the making since the hackathon at Magento Live 2019 in Amsterdam, thank you to everybody involved with this 🎉 🙌

---

I'm curious how this will perform, we've been using the alternative solution to this problem, using the following PR since 2021 on all our shops running Magento 2.4.x and haven't seen any issues with it so far.

---

Also, the new static_content_only_enabled_modules config setting in the deployment config should be documented, can somebody take care of that?

[engcom-Charlie]

Hi @hostep,

Thats right, This PR got merge in ada8d1d.

The PR status is Open as few commits are not part of merged PR at the time of acceptance. I will be adding these remaining commits in next Mainline PR till then requesting for not to close it.

Thank you!

[ihor-sviziev]

Finally it marked as merged! Thank you!