Skip to content

Commit

Permalink
STRF-4603: Webpack 4 + Javascript optimizations
Browse files Browse the repository at this point in the history
* Upgrade to webpack 4
* Clean up stencil bootstrap
* Simplify interface for PageManager and get rid of async library dependency
* Get rid of html5-history-api library dependency (no longer needed)
* Get rid of fastclick library dependency (no longer needed)
* Upgrade several babel dependencies
* Separate webpack config into common, dev, prod, and add npm scripts to build
* Get rid of minifications in babel loader, instead rely on webpack
* Get rid of LoaderOptionsPlugin
* Get rid of CommonsChunkPlugin (webpack 4 will automaticaly do this for prod)
  • Loading branch information
mattolson committed May 17, 2018
1 parent 3d4d715 commit 31bdddb
Show file tree
Hide file tree
Showing 22 changed files with 97 additions and 161 deletions.
14 changes: 7 additions & 7 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
language: node_js
node_js:
- 4
- 6
sudo: required
dist: trusty
install:
- bundle install
- npm install -g grunt-cli
- npm install -g bigcommerce/stencil-cli
- npm install
- bundle install
- npm install -g grunt-cli
- npm install -g bigcommerce/stencil-cli
- npm install
script:
- grunt check
- stencil bundle
- grunt check
- stencil bundle
30 changes: 7 additions & 23 deletions assets/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,32 +63,16 @@ window.stencilBootstrap = function stencilBootstrap(pageType, contextJSON = null
return {
load() {
$(async () => {
let globalClass;
let pageClass;
let PageClass;

// Finds the appropriate class from the pageType.
const pageClassImporter = pageClasses[pageType];
if (typeof pageClassImporter === 'function') {
PageClass = (await pageClassImporter()).default;
}

// Load globals
if (loadGlobal) {
globalClass = new Global();
globalClass.context = context;
Global.load(context);
}

if (PageClass) {
pageClass = new PageClass(context);
pageClass.context = context;
}

if (globalClass) {
globalClass.load();
}

if (pageClass) {
pageClass.load();
// Find the appropriate page loader based on pageType
const pageClassImporter = pageClasses[pageType];
if (typeof pageClassImporter === 'function') {
const PageClass = (await pageClassImporter()).default;
PageClass.load(context);
}
});
},
Expand Down
10 changes: 4 additions & 6 deletions assets/js/theme/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import { classifyForm, Validators, insertStateHiddenField } from './common/form-
import swal from 'sweetalert2';

export default class Account extends PageManager {
constructor() {
super();
constructor(context) {
super(context);

this.$state = $('[data-field-type="State"]');
this.$body = $('body');
}

loaded(next) {
onReady() {
const $editAccountForm = classifyForm('form[data-edit-account-form]');
const $addressForm = classifyForm('form[data-address-form]');
const $inboxForm = classifyForm('form[data-inbox-form]');
Expand All @@ -27,7 +27,7 @@ export default class Account extends PageManager {
this.passwordRequirements = this.context.passwordRequirements;

// Instantiates wish list JS
this.wishlist = new Wishlist();
Wishlist.load();

This comment has been minimized.

Copy link
@jbruni

jbruni May 18, 2018

Contributor

@mattolson - Shouldn't it be Wishlist.load(this.context); ?

This comment has been minimized.

Copy link
@mattolson

mattolson May 18, 2018

Author Contributor

load is a static method that is bound to the same context as referenced by this object, so you can just call it directly. 31bdddb#diff-8b95a20e21550730e7b5557f56908c73R75


if ($editAccountForm.length) {
this.registerEditAccountValidation($editAccountForm);
Expand Down Expand Up @@ -67,8 +67,6 @@ export default class Account extends PageManager {
}

this.bindDeleteAddress();

next();
}

/**
Expand Down
9 changes: 3 additions & 6 deletions assets/js/theme/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import forms from './common/models/forms';
import { classifyForm, Validators } from './common/form-utils';

export default class Auth extends PageManager {
constructor() {
super();
constructor(context) {
super(context);
this.formCreateSelector = 'form[data-create-account-form]';
}

Expand Down Expand Up @@ -167,9 +167,8 @@ export default class Auth extends PageManager {

/**
* Request is made in this function to the remote endpoint and pulls back the states for country.
* @param next
*/
loaded(next) {
onReady() {
const $createAccountForm = classifyForm(this.formCreateSelector);
const $loginForm = classifyForm('.login-form');
const $forgotPasswordForm = classifyForm('.forgot-password-form');
Expand All @@ -193,7 +192,5 @@ export default class Auth extends PageManager {
if ($createAccountForm.length) {
this.registerCreateAccountValidator($createAccountForm);
}

next();
}
}
2 changes: 1 addition & 1 deletion assets/js/theme/brand.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import $ from 'jquery';
import FacetedSearch from './common/faceted-search';

export default class Brand extends CatalogPage {
loaded() {
onReady() {
if ($('#facetedSearch').length > 0) {
this.initFacetedSearch();
} else {
Expand Down
4 changes: 1 addition & 3 deletions assets/js/theme/cart.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ import { defaultModal } from './global/modal';
import swal from 'sweetalert2';

export default class Cart extends PageManager {
loaded(next) {
onReady() {
this.$cartContent = $('[data-cart-content]');
this.$cartMessages = $('[data-cart-status]');
this.$cartTotals = $('[data-cart-totals]');
this.$overlay = $('[data-cart] .loadingOverlay')
.hide(); // TODO: temporary until roper pulls in his cart components

this.bindEvents();

next();
}

cartUpdate($target) {
Expand Down
2 changes: 1 addition & 1 deletion assets/js/theme/category.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import $ from 'jquery';
import FacetedSearch from './common/faceted-search';

export default class Category extends CatalogPage {
loaded() {
onReady() {
if ($('#facetedSearch').length > 0) {
this.initFacetedSearch();
} else {
Expand Down
3 changes: 2 additions & 1 deletion assets/js/theme/common/product-details.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export default class ProductDetails {
this.imageGallery.init();
this.listenQuantityChange();
this.initRadioAttributes();
this.wishlist = new Wishlist().load();

Wishlist.load();

This comment has been minimized.

Copy link
@jbruni

jbruni May 18, 2018

Contributor

@mattolson - Same thing here. (Shouldn't it be Wishlist.load(this.context); ?)

This comment has been minimized.

Copy link
@mattolson

mattolson May 18, 2018

Author Contributor

load is a static method that is bound to the same context as referenced by this object, so you can just call it directly. 31bdddb#diff-8b95a20e21550730e7b5557f56908c73R75

This comment has been minimized.

Copy link
@jbruni

jbruni May 20, 2018

Contributor

@mattolson - Yes, I see load static method definition at the base PageManager class. It expects context as parameter.

The code you linked in your comment (31bdddb#diff-8b95a20e21550730e7b5557f56908c73R75) is creating an instance, using the load static method, and it is passing the context parameter.

Here, at this line we are commenting at GitHub, the Wishlist class instance will be created by the static method, but this.context, inside it, will be undefined, because load is being called without arguments.

This will usually not be an issue, because this.context is almost not used inside the WishList class. It is used only once, here:

const confirmed = window.confirm(this.context.wishlistDelete);

This code snippet is normally only activated from the account > wishlist page, where the Wishlist class is the main page class.

But here, the line being discussed is at assets/js/theme/common/product-details.js file, right?

So... to understand clearly what is the issue, you can add this code as the first line of templates/components/products/product-view.html:

<button data-wishlist-delete>Delete All Wishlists</button>

Then, visit any product page, and click this "Delete All Wishlists" button. You will see the following in console:

selection_257

What is going on? Like I said, this.context is undefined inside the WishList class instance.

Why? Because load has been called without any parameters. But load expects context as parameter. It is missing.

In default Cornerstone, this won't be an issue, because, as said above, this.context is almost not used (in WishList class). The only time it is used, it is in a scenario where this.context is available. But any customization may expect to have this.context with the context in there in all scenarios (product page, account page).

Please let me know if my point is clear now.


const $form = $('form[data-cart-item-add]', $scope);
const $productOptionsElement = $('[data-product-option-change]', $form);
Expand Down
2 changes: 1 addition & 1 deletion assets/js/theme/compare.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import $ from 'jquery';
import swal from 'sweetalert2';

export default class Compare extends PageManager {
loaded() {
onReady() {
const message = this.context.compareRemoveMessage;

$('body').on('click', '[data-comparison-remove]', event => {
Expand Down
2 changes: 1 addition & 1 deletion assets/js/theme/contact-us.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import $ from 'jquery';
import forms from './common/models/forms';

export default class ContactUs extends PageManager {
loaded() {
onReady() {
this.registerContactFormValidation();
}

Expand Down
3 changes: 1 addition & 2 deletions assets/js/theme/gift-certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import { defaultModal } from './global/modal';

export default class GiftCertificate extends PageManager {
constructor(context) {
super();
this.context = context;
super(context);

const $certBalanceForm = $('#gift-certificate-balance');

Expand Down
15 changes: 1 addition & 14 deletions assets/js/theme/global.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import $ from 'jquery';
import './common/select-option-plugin';
import 'html5-history-api';
import PageManager from './page-manager';
import quickSearch from './global/quick-search';
import currencySelector from './global/currency-selector';
Expand All @@ -15,21 +14,10 @@ import maintenanceMode from './global/maintenanceMode';
import carousel from './common/carousel';
import 'lazysizes';
import loadingProgressBar from './global/loading-progress-bar';
import FastClick from 'fastclick';
import sweetAlert from './global/sweet-alert';

function fastClick(element) {
return new FastClick(element);
}

export default class Global extends PageManager {
/**
* You can wrap the execution in this method with an asynchronous function map using the async library
* if your global modules need async callback handling.
* @param next
*/
loaded(next) {
fastClick(document.body);
onReady() {
quickSearch();
currencySelector();
foundation($(document));
Expand All @@ -43,6 +31,5 @@ export default class Global extends PageManager {
maintenanceMode(this.context.maintenanceMode);
loadingProgressBar();
sweetAlert();
next();
}
}
29 changes: 8 additions & 21 deletions assets/js/theme/page-manager.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,22 @@
import async from 'async';
import $ from 'jquery';

export default class PageManager {
constructor(context) {
this.context = context;
}

before(next) {
next();
}

loaded(next) {
next();
type() {
return this.constructor.name;
}

after(next) {
next();
onReady() {
}

type() {
return this.constructor.name;
}
static load(context) {
const page = new this(context);

load() {
async.series([
this.before.bind(this), // Executed first after constructor()
this.loaded.bind(this), // Main module logic
this.after.bind(this), // Clean up method that can be overridden for cleanup.
], (err) => {
if (err) {
throw new Error(err);
}
$(document).ready(() => {
page.onReady.bind(page)();
});
}
}
12 changes: 1 addition & 11 deletions assets/js/theme/product.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,14 @@ export default class Product extends PageManager {
this.$reviewLink = $('[data-reveal-id="modal-review-form"]');
}

before(next) {
onReady() {
// Listen for foundation modal close events to sanitize URL after review.
$(document).on('close.fndtn.reveal', () => {
if (this.url.indexOf('#write_review') !== -1 && typeof window.history.replaceState === 'function') {
window.history.replaceState(null, document.title, window.location.pathname);
}
});

next();
}

loaded(next) {
let validator;

// Init collapsible
Expand All @@ -53,13 +49,7 @@ export default class Product extends PageManager {
return false;
});

next();
}

after(next) {
this.productReviewHandler();

next();
}

productReviewHandler() {
Expand Down
2 changes: 1 addition & 1 deletion assets/js/theme/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default class Search extends CatalogPage {
urlUtils.goToUrl(url);
}

loaded() {
onReady() {
const $searchForm = $('[data-advanced-search-form]');
const $categoryTreeContainer = $searchForm.find('[data-search-category-tree]');
const url = Url.parse(window.location.href, true);
Expand Down
11 changes: 3 additions & 8 deletions assets/js/theme/wishlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { api } from '@bigcommerce/stencil-utils';
import { defaultModal } from './global/modal';

export default class WishList extends PageManager {
constructor() {
super();
constructor(context) {
super(context);

this.options = {
template: 'account/add-wishlist',
Expand Down Expand Up @@ -83,7 +83,7 @@ export default class WishList extends PageManager {
});
}

load() {
onReady() {
const $addWishListForm = $('.wishlist-form');

if ($addWishListForm.length) {
Expand All @@ -93,9 +93,4 @@ export default class WishList extends PageManager {
this.wishlistDeleteConfirm();
this.wishListHandler();
}

loaded(next) {
this.load();
next();
}
}
2 changes: 1 addition & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var webpackTestConfig = require('./webpack.conf.js');
var webpackTestConfig = require('./webpack.dev.js');

module.exports = function (config) {
config.set({
Expand Down
Loading

0 comments on commit 31bdddb

Please sign in to comment.