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

Identify min vers of assets that reside outside of the $relative_asse_path #10

Merged
merged 5 commits into from
Jun 5, 2024

Conversation

dpanta94
Copy link
Member

@dpanta94 dpanta94 commented Jun 5, 2024

You can reproduce the issue being fixed with any resource that resides outside of the configured relative path.

For example the resource in common vendor/jquery-tribe-timepicker/jquery.timepicker.js.

The specific version is being shipped without the unminified version and the minified would fail to be detected. the reason is that the regex replace is dependent on the asset residing within the configured relative asset path. In the case of common thats src/resources

Evaluating if the asset relative path is different from the min asset path, i think its enough in this case. In case they were different that must at the very least means that the tribe_asset function has evolved to support different locations of min vs non-min. At that point, it should probably support different relative paths per resources or we would have refactored every tribe_asset call to account for different group of resources.

I can add tests, just wanted to see if we are ok initially with the patch before moving forward.

tests/wpunit/AssetsTest.php Outdated Show resolved Hide resolved
tests/wpunit/AssetsTest.php Outdated Show resolved Hide resolved
tests/wpunit/AssetsTest.php Outdated Show resolved Hide resolved
@lucatume lucatume merged commit 3e097e1 into main Jun 5, 2024
3 checks passed
@lucatume lucatume deleted the fix/undentified-min-non-min-versions branch June 5, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants