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

Change WordPress.WP.EnqueuedResourceParameters.MissingVersion to warning #1472

Closed
GaryJones opened this issue Aug 29, 2018 · 7 comments · Fixed by #2146
Closed

Change WordPress.WP.EnqueuedResourceParameters.MissingVersion to warning #1472

GaryJones opened this issue Aug 29, 2018 · 7 comments · Fixed by #2146

Comments

@GaryJones
Copy link
Member

EnqueuedResourceParameters is included in WordPress-Extra.

It was recently added via #1378.

While I agree that it's a best practice to add a script version (WP will use its own version anyway if not given), I think we should reconsider this being reported as an error.

Example case, would be registering a script from a CDN e.g. https://cdn.bootcss.com/backgrid.js/0.3.8/backgrid.min.js - this already contains the version number, so having to add a version number as a separate argument doesn't make sense.

The arg is optional - if it was something that someone must do (for which not doing would be an error), then it wouldn't be optional.

I realise this is for Extra, not Core, and that there are ways for individual projects to ignore, disable or change the type for this violation, but I'd like to see us re-consider changing it in WPCS.

@XedinUnknown
Copy link

I would like to add that adding parameters to the URL may disable resource caching on the CDN, or even break the URL altogether.

@jrfnl
Copy link
Member

jrfnl commented Aug 29, 2018

👍 I think that would be good change.
I've also seen this issue crop up with projects using WebPack which automatically adds a hash which changes when the code changes (or something along those lines, I don't know the fine detail of WebPack), but the effect of that is that the version number isn't needed in those cases either.

@dingo-d
Copy link
Member

dingo-d commented Aug 29, 2018

It was this issue: WPTT/WPThemeReview#170

@jrfnl
Copy link
Member

jrfnl commented Nov 13, 2018

@moorscode As you did most of the work on this sniff, any thoughts ?

@CamelKing
Copy link

Any update on this? The boiler plate generated by Underscores failed phpcs, LOL

@jrfnl
Copy link
Member

jrfnl commented Mar 26, 2019

@CamelKing The _s boiler plate fails a WPCS check for a whole lot more than just this as things just aren't getting merged: Automattic/_s#1311

@jrfnl
Copy link
Member

jrfnl commented Dec 9, 2022

Not sure why nobody actioned this before, but PR #2146 should fix it now.

@jrfnl jrfnl added this to the 3.0.0 milestone Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants