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.

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

Excludes Magento_Tinymce3 scripts from the bundling, saves about 3.7 … #24477

Merged

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Sep 5, 2019

…MB in the generated JS bundle size.

Description (*)

This was discovered by accident (we normally disable the Magento_Tinymce3 module so we didn't notice this before).
Anyways, the Magento_Tinymce3 module was introduced in Magento 2.3.0, so this is probably already going on since then.
Apparently it was forgotten to exclude the javascripts of that module from the JS Bundling.

The JS bundle files took a total of about 6.9 MB and consisted of 8 files.
With this added exclusion, the bundled files are reduced to 3.2 MB and only consist of 4 files.

Fixed Issues (if relevant)

None that I could find

Manual testing scenarios (*)

  1. Enable JS bundling
  2. Enable JS minifying (probably not necessary, but I used this for testing)
  3. Put shop in production mode
  4. Run static content deploy
  5. Watch the files in pub/static/frontend/Magento/blank/en_US/js/bundle/ and pub/static/frontend/Magento/luma/en_US/js/bundle/

Somebody should probably also test if when you switch from TinyMCE4 to TinyMCE3 everything still works as expected, this is found in the configuration under General > Content Management > WYSIWYG Options > WYSIWYG Editor (I didn't go this far to test this).

Questions or comments

Stuff like this should get caught automatically somehow, if in one release the bundle size suddenly increases significantly, this should raise some flags ...

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
Copy link

m2-assistant bot commented Sep 5, 2019

Hi @hostep. 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.3-develop instance - deploy vanilla Magento instance

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

@ghost ghost assigned sidolov Sep 5, 2019
@sidolov sidolov added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Sep 5, 2019
@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5794 has been created to process this Pull Request
✳️ @sidolov, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@hostep
Copy link
Contributor Author

hostep commented Sep 7, 2019

@sidolov: be aware, I've just pushed another fix in this PR, to also exclude those files from the Magento/backend theme, since it should also be fixed over there. The backend theme bundle size goes from 7.2 MB => 3.4 MB because of this.

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5794 has been created to process this Pull Request

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@magento-engcom-team magento-engcom-team merged commit 4751251 into magento:2.3-develop Sep 10, 2019
@m2-assistant
Copy link

m2-assistant bot commented Sep 10, 2019

Hi @hostep, 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.

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.4 milestone Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Design/Frontend Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Progress: accept Release Line: 2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants