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

Prelude and jQuery #100

Closed
jeremyescott opened this issue Mar 25, 2017 · 5 comments
Closed

Prelude and jQuery #100

jeremyescott opened this issue Mar 25, 2017 · 5 comments

Comments

@jeremyescott
Copy link

I don't know if this so much an issue as a question/concern.

Prelude replaces core WP jQuery. A lot of people see that as a huge no-no. eg: https://pippinsplugins.com/why-loading-your-own-jquery-is-irresponsible/

I see that the wrap for using the Google library version checks for admin, but that causes me to pause and consider, "why then?" If js must be written for two versions of jQuery, why not allow the default library to load?

I'd love to hear your thoughts on this.

@erwstout
Copy link
Member

Good looking out. I think, to be honest, its just a relic of when we had less of a handle on developing for WordPress back a few years ago and just never had any issues with it (developing in a "bubble" will do that to you). How it works now, is that if you're not an admin it loads the google version. Which is/was what we wanted at the time, but I can now see how this could negatively impact plugin developers. (Most of our plugins we do build only do front-end work).

I'm going to take a look at removing this on Monday - and run some tests, but I think it's safe to say that by the end of the week it will be removed. Not sure if @mattada or @bebaps have any other input on the matter.

@erwstout
Copy link
Member

Also, congrats (??) on opening issue #100! :-)

@jeremyescott
Copy link
Author

I'll celebrate #100 in the only way I know best... beer!

@bebaps
Copy link
Contributor

bebaps commented Mar 27, 2017

This was in the codebase for as long as I have been here, but yes it is not needed. I can guess that it was done just to use an updated version of jQuery. WordPress suggests not using a CDN or external link to pull jQuery anyway, so if you do need an updated version in the theme just include it in the JS build process.

I say get rid of it, but just be sure to check jQuery versions needed for any third party plugins in use (Slick, Remodal, etc). An updated version of jQuery can be added as a dependency in the package.json but leave it up to the developer to use it.

@erwstout
Copy link
Member

Boom. Done. Just make sure when adding scripts that rely on jQuery its in the deps array such as what we now do by default:

wp_enqueue_script('prelude-js', get_template_directory_uri() . '/assets/js/theme.min.js', array('jquery'), THEME_VERSION, true );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants