So please let me know if this is something this project is interested in and I'll polish up my PR.

If the filesize differences are accurate I would love for this to be polished ... Reduced file size is crucial.

I'll have some questions once I have time to digest the info, but definitely excited and interested in this. Thanks!!

So I just realized that the way that dependency Tether is distributed causes some issues with the dead tree shaking. My analysis can be found here.

So that means that the final builds are slightly bigger. Worst case all of Tether is missing and that means +25kB so 138kB. Still a significant win 👍

I'll be looking more into that. ⚠️ Current Rollup builds are broken.

I actually found the issue by having the tests run against the bundled version of Reactstrap. This is something that we might want to consider doing anyway as it means that we're confident that what we ship is correct, not just our source. (but out of scope for this PR)

I did see that issue previously, someone had an issue with reactstrap and turned out to be an issue upstream with tether. Couple of things, I'm not 100% committed to tether because of it's size and how long it takes to get a response (I'm not so great at response times either :). Is it worth forking to reactstrap org or personal repo with the required rollup changes until it gets added by hubspot? Another thing to note, I'd like to move to popper.js so a temporary fork of tether doesn't seem so bad to me.

I actually found the issue by having the tests run against the bundled version of Reactstrap. This is something that we might want to consider doing anyway as it means that we're confident that what we ship is correct, not just our source. (but out of scope for this PR)

I've thought about this before, but couldn't find an example. Do you happen to have a link to a project that does this?

I like the way you think.

There already is a PR for Tether that does all the work we need (it just never got merged 😞 ): shipshapecode/tether#175. It's mergable so we should be able to get a fork going of the latest version pretty easily.

An example of a project that tests against compiled output is SvelteJS: https://github.com/sveltejs/svelte/blob/master/test/helpers.js#L11-L15

Some more questions related to this PR:

1. Do you want to keep the individual transpiled files around? Allowing people to use it might lead to people getting huge bundle sizes. This will be a breaking change.

And some questions for follow up PRs once this one gets merged: (you can ignore these if you don't have time)

2. Should we keep Webpack around? It is currently used for the docs but we could bundle that with Rollup too. That way we don't have to maintain different build configs.
3. Why still depend on the react-scripts for testing instead of depending just on jest?
4. Would it be an idea to use Yarn to make sure that we can get reproducible builds because of the dependency lock file

1. I thought most build tools ignore transpiling code in node_modules. Read a little about rollup,jsnext:main and module in package.json. They sort of mention similar ideas/warnings.

2. It is currently used for the docs but we could bundle that with Rollup too - would there be an issue with static-site-generator-webpack-plugin? I'm interested in a less complicated setup for static sites using create-react-app similar to the way component-template docs work. Also interested in something like gatsby.

3. Part of 2, component-template was built after this repo and after create-react-app came out. It seemed promising to have less config to worry about. Held off on transitioning until react router 4 was released. I need to update component-template before committing to that pattern here. If it's causing issues, I'm happy to just use what is required and add it back if needed.

4. +1 to yarn. I've used it before for app deps but not for packages that get published. Anything to look out for there? (I think I read a tweet about that being an issue but it's unverified :)

1. It is included in the bundle and so people might have depended on importing it

2. Okay, let's hold off on this for now. I'm not familiar with static site generator and don't feel like rebuilding a whole site (just yet ;-) )

3. It's not causing issues (well, after I installed watchman on my Mac). It just seemed like a weird pattern to use react-scripts because it just calls jest.

4. It's not so much for the normal dependencies but more for the dev dependencies. If a dependency pushes out a new version and forgets how to semver, all builds could start failing.

@eddywashere wrote in #359 (comment)

I'll have some questions once I have time to digest the info, but definitely excited and interested in this. Thanks!!

Any update on this? At least I will have to point package.json at a forked Tether. But what about other things ?

I have fixed the build and updated the build sizes in the PR msg.

Score! Just realized that we run Babeli on individual files instead of on the final result so I managed to get the build even smaller 👍

(and tests still pass)

This is awesome. We're using Rollup and Reactstrap for projects too, came to the same conclusion you did @balloob. Would love to help if there's more work that needs to be done, otherwise this PR has a huge +1 from me.

No more work to be done, I think that this PR is all good to go 👍

@balloob Do you have more stats on file size changes now that this is complete? Before & after? Will this impact existing users importing this library using different programs - e.g. Webpack, Browserify, etc.?

I have a list of all the benefits in the PR intro.

Just to confirm ... the latest screenshot is with the last changes in the PR ... so we're dropping from ~110KB -> ~27KB for ES ?

The latest screenshot is with gzip applied.

Without gzip we're going from 295kB to 111kB. With gzip it's 70.4kB to 27kB.

Not to be merged yet, I am still doing some tests if the final builds work in other projects. (before this I only ran the tests and verified that those passed)

If other people want to test in their projects, it's easy using Yarn.

• check out my branch of Reactstrap
• run yarn link
• run yarn build
• Go to folder of your other project
• run yarn link reactstrap

Now your project will use the reactstrap folder where you ran yarn link.

It seems tether is still giving some issues 🤔 sigh.

I am using the Tether build from the browserify build of Tether that I found. But it seems to not work 😢

I'm using that same Tether build, it's working for me. Let me pull your changes and compare.

Sigh, never mind, I was stupid. Looks like we're all good. I still had build artifacts in the lib dir and didn't pull down the package.json pointing at dist. So I was testing it against an older broken build that was not using the right Tether dependency.

If people are interested in what is taking up the space, a break down of the bundle can be found here: https://sourcemap-reactstrap-ynraqillzu.now.sh

Tether broke the source map tool, so lodash.isfunction is actually 2kB and the other 25kB is Tether.

Generated this by doing the following:

• Add sourceMap: true to the rollup config
• Run yarn build to generate a new build including source maps
• Installed source map explorer: yarn add source-map-explorer
• Explore source map: ./node_modules/.bin/source-map-explorer dist/reactstrap.cjs.js

I removed the UMD because somehow the node modules did not get included 🤔 Since it was an extra feature, I'll leave it out for now.

@balloob thanks a ton for looking into this and pushing it through! I'll do a minor release soon to get it out.

This is great, thanks @balloob - though should reactstrap be using a github hash as an 'official' release dep?:

"tether": "balloob/tether#7e0d48e4ac2162e251dc2b69d7c13e6c29a5667f"

I can fork to this org and do a npm release for it.

That would be good idea @eddywashere. And @gthomas-appfolio, please comment on shipshapecode/tether#175 too, since this should be merged upstream.

Published as reactstrap-tether. Updated in 4.4.0 release branch

Just curious --- I see es and cjs files in dist. Do we have a published version that is now compatible for including in browsers for applications that need to include Reactstrap with applications that exclude Reactstrap from one's application as some prefer to include external cached copies of this.

The documentation on the website probably needs to get updated as well since it references a .min in the NPM CDN which currently does not exist. There may be other issues with the build as well.

Yeah, this broke the lib directory as well as the dist, basically all of the distribution are busted.
https://unpkg.com/[email protected]/lib/index.js should not be es, or at least there should be cjs files as well and that is where the package.json main should be pointing
I would assume the .js files are the cjs ones so importing directly from lib will work as it currently does.
You need to remember, not everyone is using rollup and/or webpack 2, common js still need to be supported as it was before.

Before:

After:

Reactstrap no longer includes unminified versions. So what used to be reactstrap.min.js is now reactstrap.cjs.js. The file package.json was updated so that if you do require('reactstrap') you will be all good.

@virgofx, can you elaborate your use case more. Which file did you used to include and how did you use that in your page?

The lib directory (which is btw very inefficient to use) should have transpiled import/export to require. That's a bug. I'll put up a PR.

@TheSharpieOne fix for lib dir containing import/export in #384

@balloob can you elaborate on how import Card from 'reactstrap/lib/Card'; is inefficient compared to importing everything as import { Card } from 'reactstrap'; when using things that do not have tree shaking (because not everyone has jumped to that yet)?

I think umd is needed in the dist directory. Thus was @vigrofx use case, using script tags to import from the cdn. The cjs file in the dist is probably not really a needed as the lib/index.js is where cjs imports should be getting the files from.

@TheSharpieOne from the PR intro:

The individual transpiled components are great to cherry pick what you need into your project. However because they are individually transpiled, each of them will contain the Babel helpers which adds up quickly when importing multiple components. For example the transpiled Button component consists for almost 50% of Babel helpers.

I'll have a PR to add back UMD shortly. (and it's another 15kB smaller 🥂 )

Thanks @balloob Yes ... use case was just a simple HTML doc test using say one react component... application is compiled with Webpack2 using external references to React + Reactstrap ... injected as follows:

<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react/15.4.2/react-with-addons.min.js"></script>
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react/15.4.2/react-dom.min.js"></script>
<script type="text/javascript" src="//unpkg.com/[email protected]/dist/reactstrap.min.js"></script>

changing for 4.4.0:

<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react/15.4.2/react-with-addons.min.js"></script>
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react/15.4.2/react-dom.min.js"></script>
<!-- error in screenshot above -->
<script type="text/javascript" src="//unpkg.com/[email protected]/dist/reactstrap.es.js"></script> 

And then of course adding my custom bundle afterwards.

The above error was when importing the reacstrap.es (same error but for require when using cjs version). Including the unminified is not a huge priority ... just the basic minified/transpiled version to include above.

Would loose mode help shrink the individual generated lib files and/or the dist/ files?