htmlburger / wpemerge-theme

An organized, ES6 and SASS powered theme taking advantage of the WP Emerge framework. 🚀
https://wpemerge.com/
GNU General Public License v2.0
286 stars 33 forks source link

Assets::generateFileVersion() does not work with Bedrock installations #40

Closed sybbear closed 4 years ago

sybbear commented 4 years ago

Version

Expected behavior

wpemerge-theme-core/src/Assets/Assets::generateFileVersion() should return file versions with Bedrock Wordpress installations.

Actual behavior

The function always returns false, even that asset file exists.

Steps to reproduce (in case of a bug)

  1. Install Wordpress using bedrock
  2. Create a WP Emerge theme
  3. On page load, that theme assets are not versioned in the URL

Comments

We are using Bedrock boilerplate for wordpress projects, as it provides modern file structure, supports managing plugins, themes and localizations through composer and treats Wordpress as a dependency. Bedrock has the following project structure by default:

- project
   - web (webroot, /)
       - app
          - mu-plugins
          - plugins
          - themes
          - languages
       - wp (Wordpress folder)

The issue is that while generally WP Emerge works fine with Bedrock, the function wpemerge-theme-core/src/Assets/Assets::generateFileVersion() does not return a file version.

How to resolve (example project.com)

  1. https://github.com/htmlburger/wpemerge-theme-core/blob/0.15.0/src/Assets/Assets.php#L56 The line should use home_url instead of site_url
  2. https://github.com/htmlburger/wpemerge-theme-core/blob/0.15.0/src/Assets/Assets.php#L63 There should be an extendable way to replace home_url with project root path. The fix for our project was to remove wp/ from ABSPATH:
            // Generate the absolute path to the file
            $file_path = str_replace(
                [$home_url, '/'],
                [str_replace('wp/', '', ABSPATH), DIRECTORY_SEPARATOR],
                $src
            );

Bottom line

Could you implement a fix to this or suggest a way to override generageFileVersion ?

atanas-dev commented 4 years ago

Thank you for the detailed write-up @vikingmaster !

Due to the holidays I'm not exactly sure when I can release a fix (hopefully this weekend), but in the mean time you can use one of two work-arounds:

  1. Edit assets.php
  2. Replace enqueueScript() and enqueueStyle() usage with the equivalent from here: https://github.com/htmlburger/wpemerge-theme-core/blob/master/src/Assets/Assets.php#L110-L134
  3. Use hardcoded version numbers / filemtime() with semi-hardcoded file paths.

OR

  1. Copy-paste the offending function and apply a fix manually.
  2. Replace the usage in assets.php with your new function.
sybbear commented 4 years ago

Thank you for the hint :) Looking forward to see the fix in upcoming updates, WP Emerge rocks! 🤘

sybbear commented 4 years ago

Hey @atanas-dev Wanted to check, is there a fix planned any time soon?

atanas-dev commented 4 years ago

@vikingmaster just released 0.15.1 of htmlburger/wpemerge-theme-core which fixes the issue. To get the update use this: composer update htmlburger/wpemerge-theme-core

sybbear commented 4 years ago

Very much appreciated!