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

Overriding minified JavaScript file exclusion in i18n make-pot #174

Closed
ataylorme opened this issue Jul 22, 2019 · 10 comments
Closed

Overriding minified JavaScript file exclusion in i18n make-pot #174

ataylorme opened this issue Jul 22, 2019 · 10 comments

Comments

@ataylorme
Copy link

I am struggling with minified JavaScript files being ignored by the i18n make-pot command implementing translations in WP Rig.

The WP Rig starter theme, and other projects (e.g. the create-guten-block framework mentioned here), have both source JavaScript files and compiled, minified JavaScript files.

The JavaScript developers write is run through a build process and turned into the JavaScript files consumed by WordPress. This may mean transpiling JavaScript not yet supported by browsers, running minification, etc.

An example of our specific directory structure is:

assets/js
├── customizer.min.js
├── editor-filters.min.js
├── lazyload.min.js
├── navigation.min.js
└── src
    ├── customizer.js
    ├── editor-filters.js
    ├── lazyload.js
    └── navigation.js

Files in assets/js/src are the ones edited by developers and a gulp build process creates the files in assets/js that are used by WordPress.

Even when the gulp build process uses the keep_fnames option of uglify, which will keep all function names from being transformed, wp i18n make-pot ignores JavaScript files with .min in the name.

edit: the command I am attempting to run is wp i18n make-pot . languages/wp-rig.pot --exclude=vendor,node_modules,.git,gulp,tests,config,optional,assets/js/src.

However, these are the files I wish to generate a .pot file from as they are the ones being enqueued in WordPress, not the source files.

WP-CLI has a hard-coded exclude array for the i18n make-pot command that can not be overwritten with the --include or --exclude options.

I would like for a way to negate the exclusion of minified JavaScript files when creating the .pot file in this case (they should remain excluded by default).

Maybe a new option force_include which gets run after include and exclude?

@ataylorme
Copy link
Author

From @swissspidy

Also note that this should actually work since at least #87. Please verify that you are using the latest version of the package.

From the code in #87 it seems I should be using --include. I will test that and report back.

@ataylorme ataylorme changed the title Including minified JavaScript files in i18n make-pot Overriding minified JavaScript file exclusion in i18n make-pot Jul 22, 2019
@ataylorme
Copy link
Author

include doesn't seem to be a perfect solution as it overrides exclude

[--include=<paths>]
Comma-separated list of files and paths that should be used for string extraction. If provided, only these files and folders will be taken into account for string extraction

I wish to include all PHP files except specific directories (e.g. vendor). With include, I will need to generate a specific list of files. I cannot use the working directory as the directories I wish to exclude will be included.

So while it solves the minified JS problem it creates a new problem when attempting to include/ignore PHP files.

@ataylorme
Copy link
Author

ataylorme commented Jul 23, 2019

I've also been testing the merge option by first generating a .pot file from PHP files, using the desired exclude filters there, then running i18n make-pot again for JavaScript files by explicitly passing them to include and merging with the existing .pot file.

It seems to work for unminified files but files that have been passed through uglify, even with keep_fnames, aren't included.

Here is an example minified JS file:

"use strict";wp.domReady(function(){var e=wp.i18n.__;wp.blocks.registerBlockStyle("core/list",{name:"checkmark-list",label:e("Checkmark","wp-rig")})});

edit: here is the source file that minified JS was created from:

/**
 * File editor-filters.js.
 *
 * Modify the behavior of the block editor.
 */

wp.domReady( function() {
	const { __ } = wp.i18n;

	wp.blocks.registerBlockStyle( 'core/list', {
		name: 'checkmark-list',
		label: __( 'Checkmark', 'wp-rig' ),
	} );
} );

wp.i18n.__ is still called but not used directly, instead being mapped to the variable e. My guess is that the code matcher doesn't like this.

@ataylorme
Copy link
Author

To try and provide some better context to what we are doing, WP Rig runs as a development theme. There is a gulp workflow to compile CSS/JS, optimize images, run BrowserSync, etc.

That process has lots of dev dependencies and overhead but allows developers to work efficiently.

Once development is done there is a gulp task to bundle a production version of the theme. This production version does not contain unnecessary files, such as dev dependencies and un-minified source files. Instead, a new directory is created with just what is needed for production.

Once the production theme is built, I would like to use wp-cli to generate the .pot and .json translation files.

An additional challenge here is that wp-cli is being run as a dev-dependency via Composer is running the commands via Composer on a directory that is not the Composer root/cwd.

@swissspidy
Copy link
Member

So while it solves the minified JS problem it creates a new problem when attempting to include/ignore PHP files.

Include/excludes are based on scores since #104, which means a more exact match takes precedence.

wp.i18n.__ is still called but not used directly, instead being mapped to the variable e. My guess is that the code matcher doesn't like this.

That is absolutely correct and I think the main issue you're running into here. No string extraction tool would like this. Tools like WP-CLI's i18n make-pot do not execute code. They cannot know that e is actually wp.i18n.__.

You'd need to ensure that wp.i18n.__ is used as-is, or at least via var __ = wp.i18n.__.

You mentioned keep_fnames but in your WP Rig PR I don't see any usage of that option. Also worth noting that keep_fnames is an option for both minify and mangle in UglifyJS. Did you turn on both?

PS. you'd need to make sure to preserve translators: comments in your minified scripts as well.


NB: ideally you would run the string extraction on the source files in assets/js/src, not the minified production files. This way, translators will have the 100% correct context when they try to check the source code to see where a string is used. Looking at minified source code doesn't help there.

Unfortunately we don't have the best solution for this scenario just yet, see #127.

@ataylorme
Copy link
Author

ideally you would run the string extraction on the source files in assets/js/src, not the minified production files. This way, translators will have the 100% correct context when they try to check the source code to see where a string is used. Looking at minified source code doesn't help there.

Thank you for all the information @swissspidy. Currently, our production build does not ship source files, which would be problematic for using them as a translation source.

I am going to recommend that we completely skip file minification when WP Rig creates translation files. Currently, translation creation is opt-out but I think we will need to flip that and make it opt-in.

@cbratschi
Copy link

I just had problems extracting strings from a minified JS file too. First of all I deactivated optimization but the file was still ignored. Then I tried all kind of versions without success. After debugging for some hours I renamed the file from gutenberg.min.js to gutenberg.js and it suddenly worked.

Are some file patterns automatically excluded or are the two dots a problem?

@cbratschi
Copy link

Well, a.b.js works but a.min.js doesn't. Again, the JS content is not minified only the file name makes the difference.

@cbratschi
Copy link

Got it to work with --include="*.min.js".

@swissspidy
Copy link
Member

Are some file patterns automatically excluded or are the two dots a problem?

@cbratschi Yes, see

/**
* @var array
*/
protected $exclude = [ 'node_modules', '.git', '.svn', '.CVS', '.hg', 'vendor', 'Gruntfile.js', 'webpack.config.js', '*.min.js' ];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants