Disabeling js merge,bundle,minify does not work in admin #11270
Conversation
…set\ConfigInterface interface & made “admin” store view as the default store view on the adminhtml area
|
|
|
There are a lot of failing tests yet: https://travis-ci.org/magento/magento2/builds/284191082?utm_source=github_status&utm_medium=notification |
There was a problem hiding this comment.
@roma-glushko Thank you for your effort. Please fix failed tests, remove duplication and align implementation with backward compatibility guide
| /** | ||
| * @var RequireJsConfig | ||
| */ | ||
| protected $config; |
There was a problem hiding this comment.
private visibility is preferable for all new code.
| { | ||
| /** | ||
| * @var RequireJsConfig | ||
| */ | ||
| private $config; | ||
| protected $config; |
There was a problem hiding this comment.
What is the reason to increase property visibility?
| * @api | ||
| * @since 100.0.2 | ||
| */ | ||
| class Config extends AbstractBlock |
There was a problem hiding this comment.
This class duplicates Magento\RequireJs\Block\Html\Head\Config except $scopeType. Such variation should be implemented over object configuration at instantiation (use block arguments) not by code duplication or inheritance.
| * @return bool | ||
| */ | ||
| public function isMergeCssFiles(); | ||
| public function isMergeCssFiles($scopeType = ScopeInterface::SCOPE_STORE, $scopeCode = null); |
There was a problem hiding this comment.
This is forbidden change because of backward compatibility policy.
You may:
- introduce new interface with required arguments more
- introduce new class that will implement created interface
- inject new class to
Magento\Framework\View\Asset\Configand delegate all calls to new class with suitable arguments - declare Magento\Framework\View\Asset\Config as
@deprecated - inject new interface in addition to
Magento\Framework\View\Asset\ConfigInterfacewhere is needed (backward compatibility for modified constructor should be preserved) - do not remove
Magento\Framework\View\Asset\ConfigInterfacefrom any constructors, mark that argument/property as@deprecatedinstead
|
Hi @roma-glushko |
|
Hi @ishakhsuvarov , Yes, I'm going to finish this PR. Unfortunately, I will be able to to this in a week. Thank you! |
|
@roma-glushko Sure. Thank you for the quick update! |
|
Hi @roma-glushko Thanks! |
Description
When disabling the js, and css ,merge, minify and bundling for default scope and keeping them enabled for website and store view scope, the admin still loads minified and bundled js.
These changes does the following to fix the issue:
Magento\Framework\View\Asset\ConfigInterfacemethodsMagento\Store\App\FrontController\Plugin\DefaultStorepluginFixed Issues
Manual testing scenarios
Steps to reproduce
Expected result