• Resolved Shady Sharaf

    (@shadyvb)


    Thanks for the nice plugin, I’ve been a fan for quite sometime!

    One thing that has been quite painful recently is JSON translations, I’ve been patching the plugin since 2.4 to get it working, broke with 2.4.6, and patched it again, and finally had to patch 2.5 as well to make the plugin output all the JS strings of a bundle in the same JSON file for it to work with our Webpack setup that output a single bundle file to production.

    So, here is how I did this so others can benefit from the night I spent tracking down the required changes:

    ## the issue
    WordPress, and subsequently loco-translate, assume that JS files, where strings are extracted from, would exist as is (or minified with the same name but suffixed with .min ) on production sites, and that breaks translations for any site that uses JS bundlers to output single-file bundles different from the original file names.

    ## the solution
    – Patch the plugin to add a filter before the JSON generation logic so I can take over, and use that filter to intercept the process and output a single JSON file with all translations from all the JS files within the bundle.
    – Filter load_script_translation_file to let WordPress know where the file will exist, rather than guessing the file path based on its relative URL / md5 hash.

    I’ve documented the patch and related PHP functionality in this gist: https://gist.github.com/shadyvb/f2c243bd83569d8cf9bf78c8269cc76b , maybe it comes handy for others.

    I’ve noticed that you do not prefer pull-requests, hence why I’m posting here, so you can hopefully add some filters/actions so developers can customize the behavior of the plugin where needed instead of having to patch it like what I currently do. Specially given how popular is the use of bundlers like Webpack nowadays.

    Cheers!

Viewing 14 replies - 1 through 14 (of 14 total)
  • Plugin Author Tim W

    (@timwhitlock)

    I feel your pain. The JSON system is terrible and here to stay. I mentioned your exact issue to maintainers a couple of years ago and was completely ignored.

    Thanks for your work. I am happy to add more hooks. I had a recent request for some new file generation hooks, so will look at adding several in the next release.

    Regarding your original problem of mapping the name of .js files:

    There is also the load_script_textdomain_relative_path filter which WordPress uses at runtime and which Loco Translate borrows.

    ! Suddenly realised my implementation of this is broken, because the file check should run after the filter !

    This seems like the “proper” way to solve this. Does it work for your use case? Assuming I fix the above error.

    Thread Starter Shady Sharaf

    (@shadyvb)

    Thanks for adding the hooks in advance!

    So, WP does provide ways to override the path to where you can look, which is what I used in my gist. It’s the generation that’s broken, but I’ve not had to use WP’s native tooling to generate files, been always using Loco! (thanks for advocating for users with different use cases on core issues btw!)

    And, no, fixing the relative path would not work on its own, my specific issue with loco at this point is being able to control the generation process so I can combine all strings into one JSON file ( and also control its naming ). So adding hooks around that in loco should be fine for me.

    And _maybe_ if loco can support that natively (option to specify a single output JSON file for a specific bundle) it would be helpful to large segments of users going forward. But that’s a bit extra.

    Plugin Author Tim W

    (@timwhitlock)

    What plugin of yours can I use to test this? I want to try out some things and makes sense if we can be more specific.

    I’m not against generating a single JSON, but I dislike going against WordPress conventions even if I disagree with them. I’ll deal with the hooks first and have a think.

    Thread Starter Shady Sharaf

    (@shadyvb)

    That’s a private theme that uses that code, but you can simulate that by having different path / file names between generation and page load.

    And for following conventions, a non-default option to satisfy a growing subset of users doesn’t seem too bad to me. But yeah, hooks and maybe a guide would work as well.

    Plugin Author Tim W

    (@timwhitlock)

    OK, no problem.

    I just wanted to try out the load_script_textdomain_relative_path filter for your use case, because I fixed the glitch. I just figure that returning “prod.js” for any imaginary .js paths passed here would have the desired effect. Granted the file name would be a silly hash, but it should still produce a single file; critically the same at runtime as in generation. I’m sure you’ve tried it, but I just want to see for myself to learn about the problem.

    Plugin Author Tim W

    (@timwhitlock)

    I’m still going to add some more hooks, but I’ve made another fix which might be all you need.

    The JSON compilation process now buffers JSON in memory until they are all accounted for, then writes them to disk. This means that multiple JS files can be combined into a single JSON by filtering on load_script_textdomain_relative_path.

    You should now be able to do something like this:

    <?php
    add_filter('load_script_textdomain_relative_path', function( $ref, $url ){
      if( false !== strpos($url,'/my-plugin/scripts/') {
        $ref = 'scripts/prod.js'
      }
      return $ref;
    }, 10, 2 );

    As long as prod.js exists in the deployment, this should work for WordPress at runtime and Loco Translate at compile time, meaning only one filter should be necessary. The name of the resultant JSON can be unknown to the implementor.

    Of course you’ll have to be careful how you detect your own scripts being passed to the filter. My example is not 100% reliable because people can install into non-standard paths and the filter is not given any other information about what plugin it is.

    Plugin Author Tim W

    (@timwhitlock)

    Latest update also adds loco_compile_single_json filter. Return your JSON path to combine all .js references into this one file. This supports your preferred naming convention, but requires you keep your load_script_translation_file filter for runtime loading. Loco Translate cannot share this filter because it requires knowing the script handle.

    Use loco_compile_single_json like this:

    <?php
    add_filter('loco_compile_single_json', function( $jsonfile, $pofile, $domain ){
      if( 'MY_DOMAIN' === $domain ){
        $pathinfo = pathinfo($pofile);
        $jsonfile = $pathinfo['dirname'].'/'.$pathinfo['filename'].'.json';
      }
      return $jsonfile;
    }, 10, 3 );

    Both the above solutions (I think) allow you to do what you need with much less code and without exposing internal functions of Loco Translate that are not documented and may change without notice.

    I hope you feel this addresses your use case. Let me know here if not.

    Plugin Author Tim W

    (@timwhitlock)

    It would be good to get some feedback on this, because I’d like to get a release live early this week.

    Can you let me know if loco_compile_single_json works for you, and whether it causes any adverse conditions?

    Thread Starter Shady Sharaf

    (@shadyvb)

    Thanks for working on this, Tim.

    This does seem like it should do the trick! We have the update scheduled to be done next week, so will report back when that happens.

    I very much appreciate the prompt response and actions! And sorry for the late reply.

    Plugin Author Tim W

    (@timwhitlock)

    Quick update to say that the loco_compile_single_json filter no longer receives the third argument (text domain). There are deeper parts of the plugin that also need to use this filter where that data is not in scope.

    You will have to rely on the second argument (po file path) to establish if the file belongs to your plugin. That’s probably as simple as checking preg_match( '/^my-domain-/', basename($pofile) )

    Thread Starter Shady Sharaf

    (@shadyvb)

    Well, a filter can expose more information than what specific use cases need, callback can always take what they need and listen for less callback arguments.

    For the sake for unknown use cases, I’d probably add back that domain argument to minimize the risk and make it more user friendly.

    Plugin Author Tim W

    (@timwhitlock)

    Yes, a filter can expose more than a listener may use, but that’s not the issue. It’s the opposite. I can’t provide fewer than a listener may expect.

    Code listening on this hook needs to behave the same way for all instances of it being fired. Removing the domain from some triggers, and adding it for others is not good behaviour.

    In most cases PO paths will contain the text domain. It’s only really the Core which doesn’t do this.

    Thread Starter Shady Sharaf

    (@shadyvb)

    Ah, got it, makes sense.

    Plugin Author Tim W

    (@timwhitlock)

    These changes are live now in 2.5.1.

    https://github.com/loco/wp-loco/releases/tag/2.5.1

    I’m marking this issue as resolved. If you have any problems with the latest version then please start a new thread.

    Thanks for your input on this.

Viewing 14 replies - 1 through 14 (of 14 total)
  • The topic ‘Allowing custom handling of JSON translations’ is closed to new replies.