createBroadcast is not a function

[kentcdodds]

I really have no idea what's going on. But when I tried hooking this up with glamorous here I'm getting:

TypeError: createBroadcast is not a function
    at new ThemeProvider (dll.js:formatted:20246)
    at dll.js:formatted:9691
    at measureLifeCyclePerf (dll.js:formatted:9575)
    at ReactCompositeComponentWrapper._constructComponentWithoutOwner (dll.js:formatted:9690)
    at ReactCompositeComponentWrapper._constructComponent (dll.js:formatted:9683)
    at ReactCompositeComponentWrapper.mountComponent (dll.js:formatted:9638)
    at Object.mountComponent (dll.js:formatted:1790)
    at ReactCompositeComponentWrapper.performInitialMount (dll.js:formatted:9724)
    at ReactCompositeComponentWrapper.mountComponent (dll.js:formatted:9670)
    at Object.mountComponent (dll.js:formatted:1790)

The problem is in this file.

It's tricky debugging in codesandbox, but this is what the code looks like:

You'll see that I have a breakpoint on where we're requiring createBroadcast but that breakpoint never hits and it hits the _this.broadcast = ... line first. Quite odd. Not sure what's going on here.

[kof]

Wondering how this ever worked, because @iamstarkov is not using es6 imports, he uses require, but brcast is exporting a default using es6 export default https://github.com/vesparny/brcast/blob/master/index.js#L1 which means when importing with require one would need to write require('brcast').default

@iamstarkov I think in any case its better to use import statements.

[vesparny]

@kof brcast's main module is this one https://unpkg.com/[email protected]/dist/brcast.cjs.js

That's the reason why it was working I guess.

[kof]

Then in some way @kentcdodds is not using that main module on the playground.

[vesparny]

I just looked at the theming dist folder https://unpkg.com/[email protected]/dist/.

Files are only transpiled with babel, but dependencies are not bundled.
It's not a problem of module format, but a problem of missing modules :)

[kentcdodds]

Hmmm... The weird thing to me is that the line that requires createBroadcast doesn't ever run before the error's thrown. I think that's what's going on. Almost seems like a bug in webpack 🤔 I'm thinking that maybe using import statements would avoid the bug though...

[vesparny]

Time to plug rollup.

@iamstarkov you can take a look at this config I use on another project.

https://github.com/vesparny/rxhr/blob/master/rollup.config.js

I export cjs es and umd bundles.

Not sure umd makes sense in this case.

Maybe @kentcdodds knows more.

[vesparny]

Hmmm... The weird thing to me is that the line that requires createBroadcast doesn't ever run before the error's thrown. I think that's what's going on. Almost seems like a bug in webpack 🤔 I'm thinking that maybe using import statements would avoid the bug though...

@kentcdodds you will still miss brcast and all other dependencies because the are missing

[kentcdodds]

Time to plug rollup.

+1, it's the best for libraries 👍

glamorous has a good rollup config too.

[kentcdodds]

you will still miss brcast and all other dependencies because the are missing

You'll notice that I actually explicitly add that to my dependencies in the sandbox so it should be working. Also, it's included in the package.json of this package. I don't think that he has to bundle brcast with the package. I'm not using the UMD build in the sandbox. I'm using the main from package.json. require/import works in that environment.

[vesparny]

uhm...if theming does not bundle all the dependencies, how could it work without using webpack/browserify?

[iamstarkov]

[email protected] has been built and published the same way as 1.0.0 and it works in webpackbin https://www.webpackbin.com/bins/-Km8TglfWP84oYhDquT1

[iamstarkov]

though [email protected] doesnt. i will investigate

[iamstarkov]

well, thats odd. it works in react-styleguidist's sandbox

[GertSallaerts]

brcast exposes a "module" entry point in its package.json which exposes the ES6 code. Some bundlers (like Webpack 2) will use this entrypoint instead of "main". Which causes your require('brcast') call to get the { default: createBroadcast } result instead of the function itself.

IMO, the safest solution is for you to use import. Because this will make sure it works with both module.exports as well as ES6 exports, regardless of what entrypoint of brcast your consumers' bundlers use.

See also facebook/create-react-app#2485, they specifically stop Webpack from using that entrypoint because it seems to cause other problems as well.

[iamstarkov]

okay

[iamstarkov]

i guess, i need to expose both main and module entry points as well

[iamstarkov]

1.0.1 is ahead

[GertSallaerts]

It's complicated and I'm not very knowledgable on the subject, but form the webpack docs:

The key "main" refers to the standard from package.json, and "module" to a proposal to allow the JavaScript ecosystem upgrade to use ES2015 modules without breaking backwards compatibility.

"module" will point to a module that has ES2015 module syntax but otherwise only syntax features that browser/node supports.

So a "babel-ified" version of your package but with export instead of module.exports. (Which seems weird to me.)

Source: https://webpack.js.org/guides/author-libraries/#final-steps
More info: https://github.com/rollup/rollup/wiki/pkg.module

[kentcdodds]

Which seems weird to me

The reason is that webpack will "transpile" modules (ESM, CommonJS, AMD), but nothing else.

[kentcdodds]

You can look at what we're doing over on glamorous with Rollup. We have a bit of a complicated setup with rollup, but it means that we can have several output variations and a great API for any module system (that doesn't require people to reference .default when using the global or CJS).

[GertSallaerts]

Makes more sense to me now after reading more closely through the second link from my previous comment. Would probably be a good idea to take a similar approach to glamorous.

[vesparny]

This article's also relevant
http://2ality.com/2017/04/setting-up-multi-platform-packages.html

[GertSallaerts]

I provided a PR to get the ball rolling.

[iamstarkov]

@gertt well, i do have PR as well =) #7

[GertSallaerts]

Whoops 😀 Well, I think they're pretty much the same with different tools. Think you forgot to add the new targets in package.json though.(fixed while I was typing) And did not include a UMD build, but I wasn't even sure if it was useful to add. So close mine if you like!

[iamstarkov]

@gertt missed entry points is a good catch, added it just now

[kentcdodds]

I prefer a rollup solution as the end result will be smaller and faster.

[iamstarkov]

@kentcdodds im not quite sold by rollup for libs.

can you explain a bit more why bundling libs will make apps smaller? i think if all libs are bundled on their own, then apps' bundles will be quite huge.

Isnt it enough that package provide esm version, and then it doesn't matter at which step tree shaking will happen. am i right?

[iamstarkov]

while we all are here, can you try and verify that temporary package @iamstarkov/theming-issue-5 fixes this issue?

[iamstarkov]

to clarify about rollup: im not against rollup, i just dont understand how bundling can help more than es modules

[kentcdodds]

The biggest thing is that rollup is capable of doing what's called "scope hoisting." I don't have time to go into it now, but I found this blogpost which I think explains it :)

[kof]

Btw. webpack soon too.

[kentcdodds]

So excited!

[GertSallaerts]

If I understand correctly, the upsides of rollup are:

Tree shaking

Removes unused code from included dependencies. Which is only useful if you include the dependencies in the bundle. For example if @iamstarkov wants to provide a special browser bundle that can be loaded with a script tag instead of included in projects that use webpack/rollup/... themselves.

Scope hoisting

Moves all code into the same scope, making for a smaller bundle with less nesting etc which would also make it a bit faster.

Downside?

You can't do import createThemeProvider from "theming/dist/create-theme-provider"; anymore. However this should only be a problem for people who use theming in an app whose build process does not support tree shaking. These people will end up with a larger bundle because they need to import everything even if they only want createThemeProvider.

For me personally, most of the projects I work on would fall into the latter category. But that does not necessarily have to mean anything 😀

[GertSallaerts]

I think the upside of @iamstarkov's approach is that it leaves the choice up to the user:

• If they use rollup (or anyhting with similar features), they can get the same benefits as if theming was built with rollup
• If they use something without those features (webpack 1 is a good example I think?), they can still import only specific parts of the library if desired and not end up with the entire package

[iamstarkov]

well, both pull-requests successfully solve this issue. As @gertt mentioned consumers' apps will benefit from both if they use rollup. To add to that, i'm more familiar with babel than with rollup. And anyway, switch between babel and rollup is quite easy, so if one time in the future we will need rollup build, thats gonna be easy.

That said, i would stick to my PR. what do you think?

[kentcdodds]

Sounds fine

[iamstarkov]

v1.0.1 is out there