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

Refactor JavaScript assets, remove jQuery dependency, introduce build system #18395

Closed
mikejolley opened this Issue Jan 9, 2018 · 20 comments

Comments

Projects
None yet
@mikejolley
Member

mikejolley commented Jan 9, 2018

This is an issue to track several a refactor of core JavaScript code. The goal being:

  1. To drop reliance on jQuery (#17877)
  2. To introduce a new build system so compiled scripts/css are not stored in the GitHub repo so we have less conflicts (#18378 (comment))
  3. To refactor all admin and frontend scripts.

@valdrinkoshi @tameemsafi @belcherj @claudiulodro
Also cc'ing @jeffikus for Flexslider plans as we use that script also which relies on jQuery.

  • Remove all .min/.css files from GitHub
  • Add build script to generate minified files, with documentation. Webpack?
  • Integrate build script into core deployment scripts (@claudiosanches)
  • WIKI page for guidelines on writing JS for WooCommerce and how to use the compiler. If we choose ES6, document it here.
  • Remove jQuery from frontend scripts and refactor:
    • add-payment-method.js
    • add-to-cart-variation.js
    • add-to-cart.js
    • address-i18n.js
    • cart-fragments.js
    • cart.js
    • checkout.js
    • country-select.js
    • credit-card-form.js
    • geolocation.js
    • lost-password.js
    • password-strength-meter.js
    • price-slider.js
    • single-product.js
    • tokenization-form.js
    • woocommerce.js
  • For admin, start with a bundle per screen (?) and begin with a refactor of the orders screens JS.

Does this sound like a good start? Happy to get contributions from those who suggested the above - core team probably won't have capacity for this work short term but we can help with the build systems to enable it.

Refs:
#17877
#18378

@jeffikus

This comment has been minimized.

Show comment
Hide comment
@jeffikus

jeffikus Jan 9, 2018

Member

Cool, thanks for copying me in - if the intention is to completely remove jQuery support, Flexslider won't work as it is essentially a jQuery plugin.

I'm just wondering why drop jQuery support if it's in core WP? I see the speed argument in #17877 which is a fair point.

Perhaps do this in a phased approach,

  1. Remove the low hanging fruit by selecting the easy/less complex scripts and removing dependancy in them
  2. Tackle the bigger/more complex libraries later
Member

jeffikus commented Jan 9, 2018

Cool, thanks for copying me in - if the intention is to completely remove jQuery support, Flexslider won't work as it is essentially a jQuery plugin.

I'm just wondering why drop jQuery support if it's in core WP? I see the speed argument in #17877 which is a fair point.

Perhaps do this in a phased approach,

  1. Remove the low hanging fruit by selecting the easy/less complex scripts and removing dependancy in them
  2. Tackle the bigger/more complex libraries later
@jeffikus

This comment has been minimized.

Show comment
Hide comment
@jeffikus

jeffikus Jan 9, 2018

Member

Sidenote, but this could be a good time for me to think about perhaps removing jQuery dependancy from Flexslider too...

Member

jeffikus commented Jan 9, 2018

Sidenote, but this could be a good time for me to think about perhaps removing jQuery dependancy from Flexslider too...

@franticpsyx

This comment has been minimized.

Show comment
Hide comment
@franticpsyx

franticpsyx Jan 9, 2018

Member

@mikejolley wondering how backwards compatibility for things like $.fn.wc_variation_form will be maintained. There's currently a lot of 3-p code that re-uses jQuery-dependent bits like this one.

Raw JS is not my strong point -- there might be a solution to this that I'm missing right now, but in general we should ensure there's a clear, upgrade-safe path for all jQuery code that's meant to be "re-usable".

Member

franticpsyx commented Jan 9, 2018

@mikejolley wondering how backwards compatibility for things like $.fn.wc_variation_form will be maintained. There's currently a lot of 3-p code that re-uses jQuery-dependent bits like this one.

Raw JS is not my strong point -- there might be a solution to this that I'm missing right now, but in general we should ensure there's a clear, upgrade-safe path for all jQuery code that's meant to be "re-usable".

@belcherj

This comment has been minimized.

Show comment
Hide comment
@belcherj

belcherj Jan 9, 2018

Member

I think the first two tasks should be done using the current build system as an initial step. I see that you are using Travis elsewhere. Should that be used as the CI for the build?

Where do the build assets go once they are built? I propose a branch called build that contains a built WooCommerce.

That of course means that the failures in Travis would have to be remedied.

Member

belcherj commented Jan 9, 2018

I think the first two tasks should be done using the current build system as an initial step. I see that you are using Travis elsewhere. Should that be used as the CI for the build?

Where do the build assets go once they are built? I propose a branch called build that contains a built WooCommerce.

That of course means that the failures in Travis would have to be remedied.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Jan 9, 2018

Member

@belcherj Can we not build when we pull down (e.g. npm run-script build) and include this in our deployment script?

Member

mikejolley commented Jan 9, 2018

@belcherj Can we not build when we pull down (e.g. npm run-script build) and include this in our deployment script?

@tameemsafi

This comment has been minimized.

Show comment
Hide comment
@tameemsafi

tameemsafi Jan 9, 2018

@mikejolley If you would like, I would be more than happy to re-write the jQuery dependent scripts you have listed into vanilla javascript. Regarding backward compatibility, it would be possible to include certain helpers which would just wrap the new vanilla js code into a jQuery version which work the same as they do now. This way people who are using jQuery would still be able to listen for events, use the addon functions until woocommerce drops support for jQuery altogether.

tameemsafi commented Jan 9, 2018

@mikejolley If you would like, I would be more than happy to re-write the jQuery dependent scripts you have listed into vanilla javascript. Regarding backward compatibility, it would be possible to include certain helpers which would just wrap the new vanilla js code into a jQuery version which work the same as they do now. This way people who are using jQuery would still be able to listen for events, use the addon functions until woocommerce drops support for jQuery altogether.

@belcherj

This comment has been minimized.

Show comment
Hide comment
@belcherj

belcherj Jan 9, 2018

Member

@mikejolley absolutely you can do that. I do think there is benefit to running the build on PR's to make sure that the PR has not broken the build. npm start to build dev and watch for changes and npm run build for build and write to file system so that it can be zipped and deployed.

Member

belcherj commented Jan 9, 2018

@mikejolley absolutely you can do that. I do think there is benefit to running the build on PR's to make sure that the PR has not broken the build. npm start to build dev and watch for changes and npm run build for build and write to file system so that it can be zipped and deployed.

@WPprodigy

This comment has been minimized.

Show comment
Hide comment
@WPprodigy

WPprodigy Jan 10, 2018

Member

Would be cool to perhaps implement this hooking system as well: https://www.npmjs.com/package/@wordpress/hooks

It's very likely going to be in WP core (don't start reading the whole ticket, trust me): https://core.trac.wordpress.org/ticket/21170

Member

WPprodigy commented Jan 10, 2018

Would be cool to perhaps implement this hooking system as well: https://www.npmjs.com/package/@wordpress/hooks

It's very likely going to be in WP core (don't start reading the whole ticket, trust me): https://core.trac.wordpress.org/ticket/21170

@tameemsafi

This comment has been minimized.

Show comment
Hide comment
@tameemsafi

tameemsafi Jan 10, 2018

@WPprodigy Yes, I think that is a good idea. This would be just one more reason to move to webpack as the build system for javascript and then use the ES6 import/export for packages like the wordpress hooks one.

tameemsafi commented Jan 10, 2018

@WPprodigy Yes, I think that is a good idea. This would be just one more reason to move to webpack as the build system for javascript and then use the ES6 import/export for packages like the wordpress hooks one.

@claudiulodro

This comment has been minimized.

Show comment
Hide comment
@claudiulodro

claudiulodro Jan 11, 2018

Contributor

Just realized select2/selectWoo has a jQuery dependency. Posting this so I don't forget. We'll need to figure out what to do about that. Selectr might work but it's pretty ugly out of the box. If we switch, we'll have to evaluate the accessibility of whatever we decide.

EDIT: Theoretically we could rewrite selectWoo to get rid of the jQuery dependency I guess. Seems like the most dramatic solution, though.

Contributor

claudiulodro commented Jan 11, 2018

Just realized select2/selectWoo has a jQuery dependency. Posting this so I don't forget. We'll need to figure out what to do about that. Selectr might work but it's pretty ugly out of the box. If we switch, we'll have to evaluate the accessibility of whatever we decide.

EDIT: Theoretically we could rewrite selectWoo to get rid of the jQuery dependency I guess. Seems like the most dramatic solution, though.

@tameemsafi

This comment has been minimized.

Show comment
Hide comment
@tameemsafi

tameemsafi Jan 11, 2018

@claudiulodro I think an alternative to select2 would be ChoicesJS. It is written in vanilla js with ES6. It can also be used in the webpack build and can be imported using npm. A bonus is that it looks nice.

tameemsafi commented Jan 11, 2018

@claudiulodro I think an alternative to select2 would be ChoicesJS. It is written in vanilla js with ES6. It can also be used in the webpack build and can be imported using npm. A bonus is that it looks nice.

@claudiulodro

This comment has been minimized.

Show comment
Hide comment
@claudiulodro

claudiulodro Jan 12, 2018

Contributor

@tameemsafi we'll be discussing this issue in the dev chat next week if you haven't seen and would like to join. :)

Contributor

claudiulodro commented Jan 12, 2018

@tameemsafi we'll be discussing this issue in the dev chat next week if you haven't seen and would like to join. :)

@tameemsafi

This comment has been minimized.

Show comment
Hide comment
@tameemsafi

tameemsafi Jan 12, 2018

@claudiulodro Thank you, I have now signed up for the slack channel and I will join the dev chat next week.

tameemsafi commented Jan 12, 2018

@claudiulodro Thank you, I have now signed up for the slack channel and I will join the dev chat next week.

@pcfreak30

This comment has been minimized.

Show comment
Hide comment
@pcfreak30

pcfreak30 Feb 13, 2018

While I understand that jQuery may have a lot of bloat gathered over the years, i dont see ditching it as the right route nor making our "own' jQuery. Im thinking maybe a custom build of jquery with only needed components to keep everything working. There is also http://zeptojs.com.

pcfreak30 commented Feb 13, 2018

While I understand that jQuery may have a lot of bloat gathered over the years, i dont see ditching it as the right route nor making our "own' jQuery. Im thinking maybe a custom build of jquery with only needed components to keep everything working. There is also http://zeptojs.com.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Feb 13, 2018

Member

I don't see it as making our own jQuery; we're using native Javascript. Aside from selectors, we don't use too much from jQuery?

Still worth investigating.

Member

mikejolley commented Feb 13, 2018

I don't see it as making our own jQuery; we're using native Javascript. Aside from selectors, we don't use too much from jQuery?

Still worth investigating.

@nyordanov

This comment has been minimized.

Show comment
Hide comment
@nyordanov

nyordanov Feb 13, 2018

Contributor

Switching from jQuery to Zepto is kind of beside the point and may even be worse. jQuery is already used by a lot of other plugins, so there is a chance that switching to Zepto will simply add one more library to the average page. The issue that will be solved if the code is refactored to use native JS is this:

For some pages/sites, WooCommerce is the only plugin which enqueues jQuery. Using native JS for what was previously implemented with jQuery will remove 32KB of JS from the page.

Contributor

nyordanov commented Feb 13, 2018

Switching from jQuery to Zepto is kind of beside the point and may even be worse. jQuery is already used by a lot of other plugins, so there is a chance that switching to Zepto will simply add one more library to the average page. The issue that will be solved if the code is refactored to use native JS is this:

For some pages/sites, WooCommerce is the only plugin which enqueues jQuery. Using native JS for what was previously implemented with jQuery will remove 32KB of JS from the page.

@pcfreak30

This comment has been minimized.

Show comment
Hide comment
@pcfreak30

pcfreak30 Feb 13, 2018

@nyordanov i sort of meant possibly force replace jQuery with Zepto in the script loader, or un-enqueue jQuery in cases were only woocommerce uses it.

pcfreak30 commented Feb 13, 2018

@nyordanov i sort of meant possibly force replace jQuery with Zepto in the script loader, or un-enqueue jQuery in cases were only woocommerce uses it.

@woocommercebot

This comment has been minimized.

Show comment
Hide comment
@woocommercebot

woocommercebot May 9, 2018

Collaborator

Thanks for the suggestion @mikejolley.

We'll keep an eye on the popularity of this request :)

Collaborator

woocommercebot commented May 9, 2018

Thanks for the suggestion @mikejolley.

We'll keep an eye on the popularity of this request :)

@jfudman

This comment has been minimized.

Show comment
Hide comment
@jfudman

jfudman May 31, 2018

Please! It's 2018. Let's get away from jQuery for crying out loud. ;)
Seriously, I'm crying out loud.

jfudman commented May 31, 2018

Please! It's 2018. Let's get away from jQuery for crying out loud. ;)
Seriously, I'm crying out loud.

@notrealdev

This comment has been minimized.

Show comment
Hide comment
@notrealdev

notrealdev Aug 29, 2018

Yes, i want this

document.addEventListener( 'added_to_cart', function() {
    console.log( 'added' );
} );

Not this

jQuery( document.body ).on( 'added_to_cart', function() {
    console.log( 'added' );
} );

Please...

notrealdev commented Aug 29, 2018

Yes, i want this

document.addEventListener( 'added_to_cart', function() {
    console.log( 'added' );
} );

Not this

jQuery( document.body ).on( 'added_to_cart', function() {
    console.log( 'added' );
} );

Please...

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