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

Reduce theme bundle size by using specific minified libraries #1037

Merged
merged 1 commit into from
Jul 6, 2017
Merged

Reduce theme bundle size by using specific minified libraries #1037

merged 1 commit into from
Jul 6, 2017

Conversation

sacr3dc0w
Copy link
Contributor

@sacr3dc0w sacr3dc0w commented Jul 6, 2017

What?

In an effort to reduce the size of the theme bundle, I've pointed two libraries to their minified versions.

Screenshots (if appropriate)

Bundle, before history.min.js
1_sourcemap-visualizer-history-before

Bundle, after history.min.js
2_sourcemap-visualizer-history-after

Bundle, before slick.min.js
3_sourcemap-visualizer-slick-before

Bundle, after slick.min.js
4_sourcemap-visualizer-slick-after

Bundle, after both history.min.js and slick.min.js
5_sourcemap-visualizer-both-after

Copy link
Contributor

@junedkazi junedkazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sacr3dc0w this is amazing. Thanks for all the work on cornerstone theme. Can you please add a change log entry ?

@PreciselyAlyss
Copy link
Contributor

@junedkazi are you comfortable with one of us merging this as it now has a changelog entry?

@PreciselyAlyss PreciselyAlyss merged commit 55b2985 into bigcommerce:master Jul 6, 2017
@junedkazi
Copy link
Contributor

@PreciselyAlyss we plan on cutting a new release today. So will merge it after that is done. Since we don't want to add any new scopes to the testing effort which is based on the current master.

@@ -1,5 +1,5 @@
import $ from 'jquery';
import 'slick-carousel/slick/slick';
import 'slick-carousel/slick/slick.min';
Copy link
Contributor

@mjschock mjschock Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sacr3dc0w - this is great! can we put them in the webpack.conf.js resolve instead:

    resolve: {
        alias: {
            html5-history-api: path.resolve(__dirname, 'node_modules/html5-history-api/history.min'),
            jquery: path.resolve(__dirname, 'node_modules/jquery/dist/jquery.min.js'),
            jstree: path.resolve(__dirname, 'node_modules/jstree/dist/jstree.min.js'),
            slick-carousel: path.resolve(__dirname, 'node_modules/slick-carousel/slick/slick.min'),
        },
    },

that way the imports can simply be:

import 'slick-carousel';
...
import 'html5-history-api'

seeing as this was already merged, feel free to open another PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sacr3dc0w i'll just go ahead and create a quick pr

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

Successfully merging this pull request may close these issues.

None yet

4 participants