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

Improving JavaScript code for performance #573

Closed
fredericalpers opened this issue Jun 20, 2023 · 9 comments · Fixed by #615
Closed

Improving JavaScript code for performance #573

fredericalpers opened this issue Jun 20, 2023 · 9 comments · Fixed by #615
Labels
QA Issue or Pull request that is in review
Milestone

Comments

@fredericalpers
Copy link
Member

fredericalpers commented Jun 20, 2023

Current state

Recently, the requests and complaints regarding the speed and general performance of the plugin have increased again. We would like to improve the performance of the plugin.

JavaScript files should be reviewed and improved in this regard.

Possible action items:

Desired state

The plugin's performance should improve.

@fredericalpers fredericalpers added this to the v4.15 milestone Jun 20, 2023
@yeneastgate
Copy link
Contributor

@fredericalpers

  1. Reducing the amount of JavaScript that is needed:
    => After checking, I see we have these 2 js files that can be merged together:
    "onoffice-duplicate-check-option-update.js" and "onoffice-warning-active-plugin-seo.js"
    => I estimate 2h to do JS file merging and functional testing. Please give me your opinion on this matter.

  2. Removing unused code
    => After checking, I see there are 2 redundant lines of code:
    Line 42 & 65 files js/ajax_settings.js
    Line 5 files js/onoffice-reference-estate-select.js
    => I need 2 hours to implement the deletion and retest the function

  3. Only load required Scripts (Only load required scripts and CSS Only load required scripts and CSS #554)
    => I estimated 6 days to finish it

  4. Minify Javascript
    => I tried implementing "Webpack minify Javascript" extension. Here are the results:
    google speed: performance has increased because after doing minify, the file size has decreased => therefore the load time will be shorter.
    image

for instance:
js/onoffice-form-preview.js 5.1Kib > dist/onoffice-form-preview.min.js 3.4 Kib
js/onoffice-multiselect.js 4.9Kib > dist/onoffice-multiselect.min.js 3.6 Kib
You can see the details in the image below:
image

=> I will choose "Webpack" as the minifile solution. Do you agree with me on this solution? If you agree then I need 6 days to implement and test all the features.

@fredericalpers
Copy link
Member Author

@yeneastgate Please go ahead and implement the suggested solutions. Thank you :)

@yeneastgate
Copy link
Contributor

@fredericalpers After consideration, we see that we don't have enough effort and time to do issue #573 this cycle, since the two issues #554 and #557 required a lot of testing effort. Could you please move this issue to the ramp-up or the next cycle so we can work on it?

@fredericalpers
Copy link
Member Author

@fredericalpers After consideration, we see that we don't have enough effort and time to do issue #573 this cycle, since the two issues #554 and #557 required a lot of testing effort. Could you please move this issue to the ramp-up or the next cycle so we can work on it?

Postpone Issue #573 to ramp up for now, if necessary we will postpone it to 4.16 :)

@yeneastgate
Copy link
Contributor

yeneastgate commented Aug 17, 2023

Hi, @fredericalpers .
As we discussed in the meeting yesterday, I suggest for issue #573, I will just implement Webpack to compress the size of current js files to the smallest possible extent.
Note: To avoid conflict with other branches
=> After you review and merge all branches of this cycle v4.15, I will replace the "src" of the current file " with the "new src" of the new mini file.
Because in issue #554 the loading position of js files has changed quite a bit.
ex:
src="http://wordpress.local/wp-content/plugins/oo-wp-plugin/js/onoffice-captchacontrol.js?ver=6.3" => src="http://wordpress.local/wp-content/plugins/oo-wp-plugin/js/onoffice-captchacontrol.min.js?ver=6.3"
Please let me know your opinion about this, thanks!

@fredericalpers
Copy link
Member Author

@yeneastgate sounds good! thank you :)

@yeneastgate
Copy link
Contributor

Hi, @fredericalpers:
A. Here is a list of files I have refactored code and mini files

  1. I merged 2 files "onoffice-duplicate-check-option-update.js" and "onoffice-warning-active-plugin-seo.js" into "onoffice-handle-notification-actions.js"

  2. I have implemented minify javascript for 23 JS files as listed below:
    image
    video demo: https://files.fm/u/yh3hxferf

B. Here are the results for performance before and after refactor
573-pagespeed

Please check and let me know your opinion about this.

@fredericalpers
Copy link
Member Author

@yeneastgate looks good to me! we will review it more in depth soon, thank you :)

@fredericalpers fredericalpers added QA Issue or Pull request that is in review and removed 1 week labels Aug 22, 2023
@yeneastgate
Copy link
Contributor

Hi, @fredericalpers. Here's a guide on how to minify file js by webpack:

  1. Run the command line "npm install" to install any packages that it depends on.
  2. Place all the ".js" files you want to minify into the directory "wp-content/plugins/nameRootOnofficePluginFolder/js".
  3. Then, run the command line "npm run build" to build the ".min.js" file.
    Once the process completes, the minified files will be located in the directory "wp-content/plugins/nameRootOnofficePluginFolder/dist".

Note: If you have added or edited any JavaScript files in the "wp-content/plugins/nameRootOnofficePluginFolder/js" folder during development, be sure to run the command "npm run build" again. This will ensure that the logic for the ".min.js" file is updated accordingly.

Video demo:

minify.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Issue or Pull request that is in review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants