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

💡 Rename wp-tinymce.js to wp-tinymce.min.js #1396

Closed
StonehengeCreations opened this issue Apr 15, 2024 · 3 comments · Fixed by #1518
Closed

💡 Rename wp-tinymce.js to wp-tinymce.min.js #1396

StonehengeCreations opened this issue Apr 15, 2024 · 3 comments · Fixed by #1518
Labels
flag: breaking change Feature will break backward compatibility. status: has pr This change has been started in a PR. type: feature request New feature or request

Comments

@StonehengeCreations
Copy link

Context

The file wp-tinymce.js, located in /wp-includes/js/tinymce/, is currently named as unminified.
Yet the content is minified, because it combines the minified versions of the scripts /wp-includes/js/tinymce/tinymce.js and /wp-includes/js/tinymce/plugins/compat3x/plugin.js into one file.

Line 98 of /wp-includes/script-loader.php determines if a compressed version should be loaded, but uses an uncompressed file extension for this.

Benefits of renaming this file:

  • Using the correct file extension to align with coding standards.
  • min.js files are excluded by default using the WP-CLI command wp i18n make-pot.
  • Prevents Memory Exhaust errors when creating a new pot file.

Possible implemantion

No response

Possible Solution

  1. Manually rename /wp-includes/js/tinymce/wp-tinymce.js to /wp-includes/js/tinymce/wp-tinymce.min.js.
  2. Minor code change in /wp-includes/script-loader.php, line 98.

From:
$scripts->add( 'wp-tinymce', includes_url( 'js/tinymce/' ) . 'wp-tinymce.js', array(), $tinymce_version );

To:
$scripts->add( 'wp-tinymce', includes_url( 'js/tinymce/' ) . 'wp-tinymce.min.js', array(), $tinymce_version );

Will you be able to help with the implementation?

signed up

@StonehengeCreations StonehengeCreations added status: needs triage This issue needs revision, splitting, or other "gardening" work type: feature request New feature or request labels Apr 15, 2024
@KTS915
Copy link
Member

KTS915 commented Apr 18, 2024

This sounds like a good idea. I just did a quick test and TinyMCE seems to work fine.

However, we can't implement this until version 3 at the earliest. That's because it would be a breaking change, i.e. plugins and themes might be relying on this script, and won't find it if we change its name. Because we are following semantic versioning, we can't make a breaking change until the next major version of ClassicPress, which will be version 3.

But then there's also the question of whether ClassicPress will even still be using TinyMCE by the time we get to version 3. The current version of TinyMCE is very old and no longer maintained. We need to decide whether to update it to a newer version or to move to an alternative editor. There have been discussions about that on Zulip, but no decision has yet been taken. Feel free to try out Tiny v6, Toast UI, Quill.js, CKEditor, and others you come across to see if you have a preference.

@KTS915 KTS915 added flag: major change Feature would need to be included in the a major version. Requires additional discussion and work. flag: breaking change Feature will break backward compatibility. and removed status: needs triage This issue needs revision, splitting, or other "gardening" work flag: major change Feature would need to be included in the a major version. Requires additional discussion and work. labels Apr 18, 2024
@StonehengeCreations
Copy link
Author

I understand your concern about plugins and themes relying on this script, of course.

But since this script is already being registered using wp_default_packages, those plugins and themes will simply use the wp-tinymce handle. So, the chance of breaking a site seems very small to me.

@mattyrob
Copy link
Collaborator

mattyrob commented Apr 30, 2024

Any change here would need to account for the build steps. wp-tinymce.js is a dynamic and built file created during core code build:

ClassicPress/Gruntfile.js

Lines 759 to 773 in be5c1e8

concat: {
tinymce: {
options: {
separator: '\n',
process(src, filepath) {
return `// Source: ${filepath.replace( BUILD_DIR, '' )}\n${src}`;
}
},
src: [
`${BUILD_DIR}wp-includes/js/tinymce/tinymce.min.js`,
`${BUILD_DIR}wp-includes/js/tinymce/themes/modern/theme.min.js`,
`${BUILD_DIR}wp-includes/js/tinymce/plugins/*/plugin.min.js`
],
dest: `${BUILD_DIR}wp-includes/js/tinymce/wp-tinymce.js`
},

There is no unminified file and added a dynamic suffix for dev environments would not work.

@mattyrob mattyrob added the status: has pr This change has been started in a PR. label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag: breaking change Feature will break backward compatibility. status: has pr This change has been started in a PR. type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants