Reactstrap: build breaks with 5.0.0-beta.2

[cvlmtg]

Issue description

• reactstrap version #5.0.0-beta.2
• import method es
• react version #16.2.0
• bootstrap version #4.0.0

What is happening?

when building with gulp + browserify this happens:

/Users/matteo/dev/my-project/node_modules/react-popper/lib/react-popper.js:1
export { default as Manager } from './Manager';
^
ParseError: 'import' and 'export' may appear only with 'sourceType: module'

• If I install 5.0.0-beta everything is fine
• If I install 5.0.0-beta.2 the build breaks
• If I manually install react-popper 0.8.2 and delete 0.8.1 everything is fine again

the strange thing is that react-popper was updated from 0.8.1 to 0.8.2 to "fix: build" (see https://github.com/souporserious/react-popper/commit/0d97929b038b1bba69f920772505f8d6af158f3f), while reactstrap's commit b515e6b says "chore(build): downgrade react-popper to fix build"

[TheSharpieOne]

The fix build was supposed to downgrade the version in the yarn lock, I didn't notice it was bumped in the package.json... Yay two package systems! ....

[TheSharpieOne]

I was going off of https://github.com/reactstrap/reactstrap/issues/859#issuecomment-368629275
@supergibbs what version of react-popper should we be depending on (to get a release out)?

[supergibbs]

0.8.1 worked for me. Hopefully there is a 0.8.3 coming soon. Fix is ready:

https://github.com/souporserious/react-popper/pull/93

[TheSharpieOne]

Right. I looked, the package.json and yarn.lock both are pinned 0.8.1, but the issue above is stating that 0.8.1 is not working, but when upgrading to 0.8.2 it does. ¯_(ツ)_/¯

[cvlmtg]

Hi, I've redone everything from scratch to be extra-sure. I can confirm react-popper 0.8.2 works for me:

$ # modified package.json to install 5.0.0-beta.2
$ rm -rf node_modules
$ npm install
$ gulp build
[...]
export { default as Manager } from './Manager';
^
ParseError: 'import' and 'export' may appear only with 'sourceType: module'
$ rm -rf build
$ npm install [email protected]
$ rm -rf node_modules/reactstrap/node_modules/react-popper
$ gulp build
[...]
[17:56:31] Finished 'build' after 59 s

[supergibbs]

I wonder if there is a babel update making that work? You aren't using yarn so the lock file is getting ignored

[supergibbs]

This works for me with npm v5.6.0 and yarn v1.5.1:

rm -rf node_modules
yarn
yarn build

running yarn add react-popper@latest installs v0.8.2 and then yarn build fails

[cvlmtg]

I'm using node 6.12.2 and npm 3.10.10, mainly because of an issue with l10ns which doesn't install on more recent node versions (at least it didn't, should check if they fixed the issue)

I'll try to find some spare time to make a small app with the minimum amount of dependencies to see if I can replicate the error, so maybe we can see if it's due to babel or something else.

[supergibbs]

Well hopefully react-popper will update soon and fix it for everyone but think this can be closed for now since yarn works (beta.2 is out 🎉) and that is the official build tool for this project.

[TheSharpieOne]

v0.8.2 doesn't have this issue: https://unpkg.com/[email protected]/lib/react-popper.js
v0.8.1 does: https://unpkg.com/[email protected]/lib/react-popper.js
Since it's complaining about export { default as Manager } from './Manager';, only the versions with that code would be an issue... right?

[supergibbs]

Oh sorry, I was confused by @cvlmtg's original issue. Using beta.2 broke their app build, not building reactstrap. I am now getting that error on my app trying to use beta.2.

Sounds like 0.8.2 breaks the rollup build in reactstrap but is needed after the build. Ugh. Maybe revert back to 0.7.4 until 0.8.3 is out (assuming that fixes it)?

[supergibbs]

Or build with 0.8.1 then change the dependency to 0.8.2?

[virgofx]

Other errors when using Reactstrap 5.0.0-beta.2 externally:

TypeError: Cannot use 'in' operator to search for 'default' in undefined

// Dist location

var ReactDOM__default = 'default' in ReactDOM ? ReactDOM['default'] : ReactDOM;

After the build need to check using Reactstrap exactly as outlined/recommended in the Readme.

[TheSharpieOne]

Do we need a postbuild test to ensure the generated files are valid or something?

[stefanb2]

I get the same issue in a project that uses Browserify + Babel for .jsx files. Hacking my node_modules/ to upgrade to react-popper 0.8.2 fixes the issue for me.

[mpolinowski]

Similar issue with a Gatsby SSR starter using reactstrap - updating to 0.8.2 solved it.

[virgofx]

@TheSharpieOne Wouldn't be a bad idea to add a quick post-build test. It would give us much more confirmation that the build will work in production (for say 50% of the cases where people include externally)

Test Setups:

1 - Using explicit dependencies

<script src="./node_modules/prop-types/prop-types.min.js"></script>
<script src="./node_modules/react/umd/react.production.min.js"></script>
<script src="./node_modules/react-dom/umd/react-dom.production.min.js"></script>
<script src="./node_modules/react-transition-group/dist/react-transition-group.min.js"></script>
<script src="./node_modules/react-popper/dist/react-popper.min.js"></script>
<script src="{{ReactstrapDistBuildDefaultMin}}"></script>

<script> // Run through a few basic reactstrap tests here </script>

2 - Using full version

<script src="./node_modules/prop-types/prop-types.min.js"></script>
<script src="./node_modules/react/umd/react.production.min.js"></script>
<script src="./node_modules/react-dom/umd/react-dom.production.min.js"></script>
<script src="{{ReactstrapDistBuildFullVersionMin}}"></script>

<script> // Run through a few basic reactstrap tests here </script>

Where the included versions are the versions specified in Reactstraps yarn.lock file (which can be copied from the node_modules folder)

[TheSharpieOne]

So, what's the plan? Are we just waiting for react-popper to release a new version? Do we want to downgrade to a version of react-popper which works both in the build and usage? Do we want to build with 0.8.1, but have the package.json reference 0.8.2?

[morloy]

For me 0.8.2 has no issues.
So the current workaround for reactstrap is to install react-popper and run rm -rf node_modules/reactstrap/node_modules/react-popper. It think updating reactstrap to 0.8.2 shoud be fine.

[TheSharpieOne]

Right, 0.8.2 doesn't appear to have any run-time issues, but it breaks the rollup build used to generate reactstrap's release artifacts due to the way [email protected] defines the exports (note in that link the exports are defined via function execution, not something that rollup's tree walking can automatically pick up).