Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add module to alert about excessive JS and CSS assets #54
Add module to alert about excessive JS and CSS assets #54
Changes from 19 commits
a7883dc
83fd785
69d1208
bda604d
663fe3a
3a5cc54
2e555a1
c668003
2baf8fe
327bed3
f7c2a60
5333d7d
b0ccda4
2757e8c
07a6a70
47f4f9f
84af518
a528fb4
e42b9c2
90c17f8
35edbca
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is okay for a start, but it's a bit oversimplified:
ABSPATH
, for example several sites have theirwp-content
directory separate, which is supported by WordPress.For most sites what you have here will work, but we should make sure to also accomodate those other sites.
I suggest we expand this as follows to at least cover for these two cases a bit better:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felixarntz can you give me an example of the first case? I cannot see it clearly.
Regarding WordPress may be in a subdirectory of the domain.
Do you mean something like this?
https://example.com/blog/wp-content/themes/test-theme/style.css ?
If that's the case, doesn't ABSPATH . wp_make_link_relative( $resource_url ) covers it too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuelRod Some popular environments like https://github.com/roots/bedrock have their
wp-content
directory outside of the (rest of the) WordPress directory, so that URLs would be e.g.https://example.com/content/themes/...
andhttps://example.com/wp/wp-includes/...
. Both conditions I'm suggesting here cover that use-case, the first is for assets that come from thewp-content
directory in a custom location, the second one for when the WordPress directory is in a subdirectory.I don't think it does, since
wp_make_link_relative
simply removes the protocol and domain. Looking at my example above, that means you get/wp/wp-includes/...
, and when you append that toABSPATH
, thewp
directory would basically be in there twice. That's why for these cases we have to use thesite_url()
function, which is the "URL equivalent" ofABSPATH
(the same way thatWP_CONTENT_URL
is the "URL equivalent" toWP_CONTENT_DIR
).