{"action":"created","issue":{"url":"https://api.github.com/repos/magento/magento2/issues/18296","repository_url":"https://api.github.com/repos/magento/magento2","labels_url":"https://api.github.com/repos/magento/magento2/issues/18296/labels{/name}","comments_url":"https://api.github.com/repos/magento/magento2/issues/18296/comments","events_url":"https://api.github.com/repos/magento/magento2/issues/18296/events","html_url":"https://github.com/magento/magento2/issues/18296","id":364580797,"node_id":"MDU6SXNzdWUzNjQ1ODA3OTc=","number":18296,"title":"Module ordering for modules without parent dependencies should be loaded in a consistent manner","user":{"login":"kirkmadera","id":1686495,"node_id":"MDQ6VXNlcjE2ODY0OTU=","avatar_url":"https://avatars0.githubusercontent.com/u/1686495?v=4","gravatar_id":"","url":"https://api.github.com/users/kirkmadera","html_url":"https://github.com/kirkmadera","followers_url":"https://api.github.com/users/kirkmadera/followers","following_url":"https://api.github.com/users/kirkmadera/following{/other_user}","gists_url":"https://api.github.com/users/kirkmadera/gists{/gist_id}","starred_url":"https://api.github.com/users/kirkmadera/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kirkmadera/subscriptions","organizations_url":"https://api.github.com/users/kirkmadera/orgs","repos_url":"https://api.github.com/users/kirkmadera/repos","events_url":"https://api.github.com/users/kirkmadera/events{/privacy}","received_events_url":"https://api.github.com/users/kirkmadera/received_events","type":"User","site_admin":false},"labels":[{"id":688811200,"node_id":"MDU6TGFiZWw2ODg4MTEyMDA=","url":"https://api.github.com/repos/magento/magento2/labels/Issue:%20Format%20is%20valid","name":"Issue: Format is valid","color":"0e8a16","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-09-27T17:36:48Z","updated_at":"2018-09-27T17:55:49Z","closed_at":null,"author_association":"NONE","body":"### Summary\r\nModule ordering for modules without parent dependencies should be loaded in a consistent manner. Currently, it is very random and can cause issues that unrelated to changes made within a release, thus making them very hard to debug.\r\n\r\n### Examples\r\nWe have hit this twice in the last month. We inherited a codebase that does not correctly set module dependencies in most of its custom modules. Everything works correctly until something causes the module order in app/etc/config.php to change. Modules tend to get rearranged significantly with any small change.\r\n\r\nExamples we ran into:\r\n* Cart page was empty because there was a module that should have depended on Magento_Checkout that didn't. This module had a bad layout override that replaced a block in checkout, but it was not a visible issue because it loaded before the Magento checkout module. It used to be included (randomly) before it; after the deploy it was included randomly after it. We ended up having to fix this by fixing the layout, but the point is that the error presented itself randomly due to unrelated changes in the release.\r\n* PDP swatch images were being rendered as very small images, then stretched causing them to be very pixellated. Same reason for the issue as above; It surfaced on a release due to random module sort order.\r\n\r\n### Proposed solution\r\nWhile declaring dependencies in module.xml files is the correct fix, this is not always possible with inherited inherited code. I think module loading could be less unreliable in this scenario if we added some consistent rules around loading modules without declared child dependencies.\r\n\r\nI propose a ruleset similar to this:\r\n\r\n1. Determine dependency order based on declared dependencies in module.xml files.\r\n2. Where there is ambiguity and modules are at the same level in the dependency tree, load them in alphabetic order.\r\n3. All modules without child dependencies should load last in alphabetic order\r\n\r\nAlphabetic ordering would prevent the random sort issues we are seeing. Modules could continue to incorrectly not have module dependencies declared but still have consistent behavior across releases except in the case where we specifically change a related module. Right now, any module change can completely reorder modules and break any of these.\r\n"},"comment":{"url":"https://api.github.com/repos/magento/magento2/issues/comments/425185775","html_url":"https://github.com/magento/magento2/issues/18296#issuecomment-425185775","issue_url":"https://api.github.com/repos/magento/magento2/issues/18296","id":425185775,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNTE4NTc3NQ==","user":{"login":"kirkmadera","id":1686495,"node_id":"MDQ6VXNlcjE2ODY0OTU=","avatar_url":"https://avatars0.githubusercontent.com/u/1686495?v=4","gravatar_id":"","url":"https://api.github.com/users/kirkmadera","html_url":"https://github.com/kirkmadera","followers_url":"https://api.github.com/users/kirkmadera/followers","following_url":"https://api.github.com/users/kirkmadera/following{/other_user}","gists_url":"https://api.github.com/users/kirkmadera/gists{/gist_id}","starred_url":"https://api.github.com/users/kirkmadera/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kirkmadera/subscriptions","organizations_url":"https://api.github.com/users/kirkmadera/orgs","repos_url":"https://api.github.com/users/kirkmadera/repos","events_url":"https://api.github.com/users/kirkmadera/events{/privacy}","received_events_url":"https://api.github.com/users/kirkmadera/received_events","type":"User","site_admin":false},"created_at":"2018-09-27T17:55:49Z","updated_at":"2018-09-27T17:55:49Z","author_association":"NONE","body":"This is hard to explain how to reproduce. You need a dirty codebase in the first place. You would need to do something like this:\r\n\r\n1. Make one or more modules that have an effect like changing layout that depends on another module loading. One example could be replacing a block in core Magento layout instead of referencing it in layout. If your module loads first, the block will be defined by your module and will be missing anything that the Magento module would load.\r\n2. Ensure that these modules do not declare their dependencies. \r\n3. You may have to manually edit app/etc/config.php to get it to load in the order which causes the failure\r\n4.  Create another test module and run setup:upgrade. This should rearrange app/etc/config.php ordering\r\n5. Confirm that your change that was showing before, no longer shows\r\n\r\nEnvironment data is not very relevant to this ticket. This will happen in both developer and production mode on any environment.\r\n\r\nThis issue would not be possible to reproduce on a vanilla instance. This is a request for an improvement to core code to help dealing with badly written custom code more consistently."}}
{"actor":{"display_login":"kirkmadera"}}