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

App Build size big due to export strategy. #1725

Closed
harshmaur opened this Issue Dec 9, 2016 · 19 comments

Comments

7 participants
@harshmaur
Copy link

harshmaur commented Dec 9, 2016

Hello,

I built my app with tree shaking using webpack 1 and I have some concerns over my build size as react instantsearch has a major chunk of it, if I combine it with algoliasearch and algoliasearch-helper.

The usual way to import is like this.

import {SearchBox} from 'react-instantsearch/dom';

This import tries to import everything that is exported from https://github.com/algolia/instantsearch.js/blob/v2/packages/react-instantsearch/dom.js

The impact of it is https://cl.ly/132r0M3H2l1n

56 KB which could have been 4KB

I changed my code from

import { InstantSearch, SearchBox } from 'react-instantsearch/dom';
import { connectInfiniteHits } from 'react-instantsearch/connectors';
import createInstantSearch from 'react-instantsearch/src/core/createInstantSearch';
import algoliasearch from 'algoliasearch';

const InstantSearch = createInstantSearch(algoliasearch, {
    Root: 'div',
    props: { className: 'ais-InstantSearch__root' },
});
import SearchBox from 'react-instantsearch/src/widgets/SearchBox.js';
import connectInfiniteHits from 'react-instantsearch/src/connectors/connectInfiniteHits.js';

and the impact was reduction of size from 113KB to 34KB.

The difference is 3x.

For someone who really needs speed in the app, every KB matters and tree shaking is not able to fix this.

It would be great if the export strategy is changed and this would help hugely with the user's build size.

You can check the export strategy of material-ui here in case that helps. https://github.com/callemall/material-ui/tree/master/src

Thanks!

@iam4x

This comment has been minimized.

Copy link
Contributor

iam4x commented Dec 9, 2016

I think tree-shaking is a feature only available with webpack2 (https://gist.github.com/sokra/27b24881210b56bbaff7#es6-specific-optimizations).

Have you tried to build your app with webpack2?

@harshmaur

This comment has been minimized.

Copy link
Author

harshmaur commented Dec 9, 2016

Okay let me try. As far as I know, tree shaking has always been there in webpack 1, they have improved it in webpack 2

@harshmaur

This comment has been minimized.

Copy link
Author

harshmaur commented Dec 9, 2016

My app has far too many complications to migrate to webpack 2. So I would prefer is there is any solution without the need to migrate.

@iam4x

This comment has been minimized.

Copy link
Contributor

iam4x commented Dec 10, 2016

That's weird tree-shaking is working with material-ui without webpack2 or rollup, see: mui-org/material-ui#5533

Anyway, react-instantsearch need another package.json entry point called modules or jsnext:main to indicate the module bundler to use another file with export style compatible with tree-shaking feature.

Will work on something without breaking the actual behaviour for older module bundler.

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Dec 14, 2016

Anyway, react-instantsearch need another package.json entry point called modules or jsnext:main to indicate the module bundler to use another file with export style compatible with tree-shaking feature.

Will work on something without breaking the actual behaviour for older module bundler.

GO!!!

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Dec 14, 2016

I know @neoziro is also working on a generic babel-plugin-lodash like able to transform any import {SearchBox} from 'some/namespace' to import SearchBox from 'some/namespace/SearchBox'. Would that help?

@harshmaur

This comment has been minimized.

Copy link
Author

harshmaur commented Dec 14, 2016

@vvo It should. Since the way I imported it in my second code decreases build size.

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Dec 14, 2016

@iam4x I am a bit lost here, if you happen to know if by default import {InstantSearch} should import only the relevant bits anytime soon (webpack 2, rollup..) that would be great. Ultimately we will need a small guide explaining how to do this given the environement, answering "How to import only what I need?"

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Dec 14, 2016

@iam4x seems material ui does something specific: https://github.com/callemall/material-ui/pull/5545/files

@iam4x

This comment has been minimized.

Copy link
Contributor

iam4x commented Dec 15, 2016

Yes @vvo it's the issue I've linked above and this is the strategy I want to use 👍 Will push something today.

@vinhlh

This comment has been minimized.

Copy link

vinhlh commented Feb 26, 2017

It seems react-instantsearch doesn't work well with webpack 2 tree shaking.

import a from './a'
import b from './b'
import c from './c'
import { SearchBox } from 'react-instantsearch/dom'

export default listing = () => {
  a()
  b()
  c()
  console.log(typeof SearchBox)
}

webpack

The overall size is quite big, 114KB gzipped. For a progressive web app, it's really big deal.

@vinhlh

This comment has been minimized.

Copy link

vinhlh commented Feb 26, 2017

importing exact used components seems to work, like @harshmaur
but have to create some boilerplate code:

import createInstantSearch from 'react-instantsearch/src/core/createInstantSearch'
import algoliasearch from 'algoliasearch'

const InstantSearch = createInstantSearch(algoliasearch, {
  Root: 'div',
  props: { className: 'ais-InstantSearch__root' }
})

export default InstantSearch

This helps to eliminate ~50KB gzipped for my case.

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Feb 28, 2017

@Haroenv Any idea how we could make things easier to allow for selective imports along with not having to re-recreate an InstantSearch?

I would go for:

@Haroenv

This comment has been minimized.

Copy link
Member

Haroenv commented Feb 28, 2017

Another idea that might work is adding a new build target that transpiles to es5, except the import/export. Then that folder should be added as module in the package.json

Older rollup supported jsnext:main, but also switched to module.

sources:

@vvo

This comment has been minimized.

Copy link
Member

vvo commented Feb 28, 2017

That's awesome, let's do that! I can handle this, I love build tools

EDIT: I did not handle it, do it

@RyanTheAllmighty

This comment has been minimized.

Copy link

RyanTheAllmighty commented Mar 16, 2017

Doing what was suggested by @harshmaur cut my app size in half (250kb).

I think this is definitely something that needs to be addressed

@Haroenv

This comment has been minimized.

Copy link
Member

Haroenv commented Mar 16, 2017

Yes, we are actively working on it, the solution suggested in the original post is the best we have for now, and will probably be the best file size we get until we do a huge effort making the other projects es2015

@mthuret

This comment has been minimized.

Copy link
Member

mthuret commented Apr 11, 2017

React InstantSearch has been moved here: https://github.com/algolia/react-instantsearch
This issue has either been rewritten or placed inside our backlog.

@mthuret mthuret closed this Apr 11, 2017

@mthuret

This comment has been minimized.

Copy link
Member

mthuret commented May 4, 2017

Some information: this issue was handle by algolia/react-instantsearch#27.
It's live with the v4.0.0-beta.6 version of react-instantsearch :)

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