Refactor to use libraries and compatibility with Big Pipe

Created on 8 March 2023, almost 2 years ago

Problem/Motivation

Right now, if big pipe is enabled, the widget can end up rendering twice. It also has caches explicitly disabled on the block which causes problems on pages where the block is used when big pipe is off (basically pages are uncacheable).

Steps to reproduce

Proposed resolution

Refactor the module to use Drupal libraries and settings to render a cacheable block and avoid using the placeholder with big pipe (since the block should be cacheable).

Remaining tasks

I've rewritten most of the module (including removing some D7 hooks which were left over). There's likely more refactoring that could be done in the build() method in the block plugin.

User interface changes

None

API changes

Most of the functions in the block plugin file were removed in favour of a library alter and settings alter. And each cdn and local js file has their own libraries.yml file entry. There is also an init js file now to set the window.gtranslateSettings variable.

Data model changes

Everything is the same as it was other than the data-* attributes on the script tag when including the gtranslate js. The domain is still set but the path is omitted (to allow caching and avoid having scripts create script tags). But the gtranslate js code already figures out the path on its own anyways so there should be no loss in functionality.

πŸ› Bug report
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @minoroffense
  • @minoroffense opened merge request.
  • πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

    Just to add a bit more clarity, because the translate js is currently not wrapped in a behavior, when the script tag gets rendered, it creates the gtranslate tag too and that code immediately runs on the page. So if something ajaxes something in or refreshes a block or region, drupal can trigger that js to run again. And so you end up with two translate drop downs.

    By allowing the block to be cached and telling Drupal to not create a placeholder (see https://git.drupalcode.org/project/gtranslate/-/merge_requests/11/diffs#...) for that block, it won't let big pipe bring it in.

    But by doing so, we want to ensure that block can be cached otherwise you break all caches on any page that block is on.

    So we remove most of the work in the block template, use libraries properly and add a settings_alter for whatever logic needed to run for that block.

  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    the MR applies cleanly.
    How can I test?

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    is there a way to include the js locally if wanted?

  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    my gtranslate Block no longer shows up in my theme.

    I have cleared cache
    there are no JS errors
    Drupal 9.4.12
    php 7.4.x

    I had to remove the Block then re-enable each time I make a change to the gtranslate settings page.

  • πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

    Probably a cache tag needs to be set or invalidated when a setting is changed if that's the case. I'll have a look.

  • πŸ‡³πŸ‡±Netherlands LaurentD

    Nice work, much cleaner code!

    Noticed some wrong mappings of the widget look options (e.g.: dropdown_with_flags) and their corresponding service names (e.g.: widget_dwf) in libraries.yml.

    Proposed change:

    cdn_widget_dropdown_with_flags:
      js:
        https://cdn.gtranslate.net/widgets/latest/dwf.js: {
          type: external,
          minified: true
        }
      dependencies:
        - gtranslate/init
    
  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    @LaurentD

    I do not see cdn_widget_dropdown_with_flags in the MR. Are you saying it should be added? If so what else should be added?

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    @minorOffense

    The gtranslate block cannot be cached. Is this correct?

    '#cache' => array('max-age' => 0), is set in gtranslate/src/Plugin/Block/GTranslateBlock.php

    Should it be cached per role or per site?
    Should per site be best since as of now, the settings do not change per user.
    I am currently using it sitewide per theme.

    This is interesting ✨ Asynchronous Google Translate Needs work

  • πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

    As far as I can tell since everything happens clientside the cache on that block should almost never expire. Only if the config changes or a CR. THe markup for the block is the same regardless.

    I removed the cache set in my merge request ( I should have remove the commented out code too so I'll have to do that).

    // '#cache' => ['max-age' => 0],

    The markup is fixed, just the attached libraries change (which Drupal takes care of separately).

    I should probably add the cache tags and contexts to the block so it clears when config changes. (I can see my notes from a month or two ago about doing so, haven't had time yet :-/)

  • πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

    @LaurentD Thanks.

    Yeah there was a lot of copy/pasting to build out that library. I may have messed some of the dependencies up. I'll check again.

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    Block is not showing after I applied the patch.
    do i need to totally uninstall.
    I did do a drush cr and even cleared cache from the UI

  • πŸ‡¨πŸ‡¦Canada minoroffense Ottawa, Canada

    The config changes made the block not show up at all with an upgrade. I had to do a full reinstall going from the 2.x to 3.x. I don't remember if I needed to do a full install between the 3.x versions.

  • πŸ‡ΊπŸ‡ΈUnited States SocialNicheGuru

    There is something wrong.
    I uninstalled and installed and it showed for a brief time.
    Then I changed the settings and it disappeared.
    Now no matter what I do I cannot get the block back.

    Maybe add the cache invalidation when the config changes would help?

    Right now, I have no gtranslate showing on my pages.

  • πŸ‡¨πŸ‡¦Canada mmaranao Calgary, AB

    Great work! I've attached the patch here while waiting to be merged.

  • πŸ‡ΊπŸ‡ΈUnited States utcwebdev

    The patch no longer applies in GTranslate 3.0.2.

  • πŸ‡¨πŸ‡¦Canada sseto

    +1 on the patch no longer applies to 3.0.3. Had to revert back to 3.0.1.

  • πŸ‡ΊπŸ‡ΈUnited States sassafrass

    We are experiencing this issue as well. I applied the patch to 3.0.1 successfully. However, as others have noted, the block content was no longer displayed. I tried completely uninstalling, deleting all blocks and config, removed the module and code completely from my code base, and then reinstalling, applying the patch, and re-configuring, but the languages still were not displayed. We reverted everything and applied a modified block preprocess function (see: https://www.drupal.org/project/gtranslate/issues/3396734#comment-15402378 πŸ› Doesn't work correctly when 2 gtranslate blocks are enabled Active ) to remove the duplicate block content being generated.

Production build 0.71.5 2024