Skip to content

Commit 1a94b0f

Browse files
virgofxTheSharpieOne
virgofx
authored and
TheSharpieOne
committed
fix(Collapse,Fade): Ensuring props don't leak to child (#598)
 and adding new production/limited dist builds. Closes #597
1 parent c090ea7 commit 1a94b0f

8 files changed

Lines changed: 192 additions & 39 deletions

File tree

‎README.md‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,65 @@ Now you are ready to use the imported reactstrap components within your componen
4747

4848
### CDN
4949

50-
Reactstrap can be included directly in your application's bundle or excluded during compilation and linked directly to a CDN.
50+
If you prefer to include Reactstrap globally by marking `reactstrap` as external in your application, the `reactstrap` library provides various single-file distributions, which are hosted on the following CDNs:
5151

52+
* [**cdnjs**](https://cdnjs.com/libraries/reactstrap)
5253
```html
53-
https://cdnjs.cloudflare.com/ajax/libs/reactstrap/4.8.0/reactstrap.min.js
54+
<!-- main version -->
55+
<script src="https://cdnjs.cloudflare.com/ajax/libs/reactstrap/5.0.0-alpha.3/reactstrap.min.js"></script>
56+
57+
<!-- all dependencies version -->
58+
<script src="https://cdnjs.cloudflare.com/ajax/libs/reactstrap/5.0.0-alpha.3/reactstrap.full.min.js"></script>
5459
```
5560

56-
> Note: When using the external CDN library, be sure to include the required dependencies as necessary **prior** to the Reactstrap library:
57-
> * [React](https://cdnjs.com/libraries/react)
61+
* [**unpkg**](https://unpkg.com/reactstrap/)
62+
```html
63+
<!-- main version -->
64+
<script src="https://unpkg.com/[email protected]/reactstrap.min.js"></script>
65+
66+
<!-- all dependencies version -->
67+
<script src="https://unpkg.com/[email protected]/reactstrap.full.min.js"></script>
68+
```
69+
70+
To load a specific version of Reactstrap replace `5.0.0-alpha.3` with the version number.
71+
72+
Reactstrap has two primary versions that can be included directly:
73+
74+
1) `reactstrap.min.js` - This file excludes `react-popper` and `react-transition-group` from
75+
the build. This is similar to Bootstrap and results in the smallest file size possible for
76+
Reactstrap. Using any components that require `Popper` or transitions will not work unless
77+
the required dependencies are included. This is the recommended approach for including
78+
Reactstrap as it allows the user to only include the optional dependencies once and re-use
79+
those dependencies in your application as well.
80+
81+
2) `reactstrap.full.min.js`
82+
This file includes all optional dependencies. (PropTypes, React, and ReactDOM must
83+
still be included)
84+
85+
#### Example
86+
87+
```html
88+
<!doctype html>
89+
<html lang="en">
90+
<head>
91+
<!-- Required dependencies -->
92+
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/prop-types/15.6.0/prop-types.min.js"></script>
93+
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react/16.0.0/umd/react.production.min.js"></script>
94+
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react-dom/16.0.0/umd/react-dom.production.min.js"></script>
95+
<!-- Most apps require transitions, so include transition dependency -->
96+
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/react-transition-group/2.2.0/react-transition-group.min.js"></script>
97+
<!-- Reactstrap -->
98+
<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/reactstrap/5.0.0-alpha.3/reactstrap.min.js"></script>
99+
<!-- Lastly, include your app's bundle
100+
<script type="text/javascript" src="/assets/bundle.js"></script>
101+
-->
102+
</head>
103+
<body>
104+
<div id="my-app" />
105+
</body>
106+
</html>
107+
108+
```
58109

59110

60111
## About the Project

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"build-docs": "cross-env BABEL_ENV=webpack WEBPACK_BUILD=production webpack --config ./webpack.dev.config.js --progress --colors",
1414
"build": "rollup -c",
1515
"prebuild": "cross-env BABEL_ENV=lib-dir babel src --out-dir lib --ignore src/__tests__/",
16+
"postbuild": "./scripts/post-build",
1617
"create-release": "npm test && sh ./scripts/release",
1718
"publish-release": "npm test && sh ./scripts/publish",
1819
"lint": "eslint src"

‎rollup.config.js‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,29 +49,56 @@ libConfig.targets = [
4949
5050
Defining this config will also check that all peer dependencies are set up
5151
correctly in the globals entry.
52+
53+
Reactstrap has two versions:
54+
55+
1) `reactstrap.min.js`
56+
This file excludes `react-popper` and `react-transition-group` from
57+
the dist build where they need to be manually required if any
58+
application uses components that require these features.
59+
60+
2) `reactstrap.full.min.js`
61+
This file includes all dependencies.
62+
5263
*/
53-
const umdConfig = baseConfig();
54-
umdConfig.external = peerDependencies;
55-
umdConfig.plugins.push(replace({
64+
const umdFullConfig = baseConfig();
65+
umdFullConfig.external = peerDependencies;
66+
umdFullConfig.plugins.push(replace({
5667
'process.env.NODE_ENV': JSON.stringify('production'),
5768
}));
58-
umdConfig.plugins.push(babili({ comments: false }));
59-
umdConfig.targets = [
60-
{ dest: 'dist/reactstrap.min.js', format: 'umd' },
69+
umdFullConfig.plugins.push(babili({ comments: false }));
70+
umdFullConfig.targets = [
71+
{ dest: 'dist/reactstrap.full.min.js', format: 'umd' },
6172
];
62-
umdConfig.globals = {
73+
umdFullConfig.globals = {
6374
react: 'React',
6475
'react-dom': 'ReactDOM',
6576
};
66-
const missingGlobals = peerDependencies.filter(dep => !(dep in umdConfig.globals));
77+
78+
const missingGlobals = peerDependencies.filter(dep => !(dep in umdFullConfig .globals));
6779
if (missingGlobals.length) {
6880
console.error('All peer dependencies need to be mentioned in globals, please update rollup.config.js.');
6981
console.error('Missing: ' + missingGlobals.join(', '));
7082
console.error('Aborting build.');
7183
process.exit(1);
7284
}
7385

86+
const external = umdFullConfig.external.slice();
87+
external.push('react-transition-group/Transition');
88+
89+
const umdConfig = Object.assign({}, umdFullConfig , {
90+
targets: [
91+
{ dest: 'dist/reactstrap.min.js', format: 'umd' },
92+
],
93+
external: external,
94+
globals: Object.assign({}, umdFullConfig.globals, {
95+
'react-transition-group/Transition': 'ReactTransitionGroup.Transition'
96+
})
97+
});
98+
99+
74100
export default [
75101
libConfig,
102+
umdFullConfig,
76103
umdConfig,
77104
];

‎scripts/post-build‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/usr/bin/env bash
2+
3+
# Ensure that any optional peer dependency (e.g. ReactTransitionGroup and
4+
# ReactPopper) are properly set as optional globals. Specifically, do not
5+
# error if the user has not loaded these dependencies. Degrade gracefully.
6+
7+
set -e
8+
9+
DIST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../dist" && pwd)"
10+
11+
# Note: Babebli transforms "global" into obfuscated/minified translated
12+
# variable names. Typically these are [a-zA-Z].
13+
sed -i -E 's#,([a-zA-Z]+)\.ReactTransitionGroup\.Transition#,\1.ReactTransitionGroup \&\& \1.ReactTransitionGroup.Transition || \{\}#' $DIST_DIR/reactstrap.min.js

‎src/CarouselItem.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from 'react';
22
import PropTypes from 'prop-types';
33
import classNames from 'classnames';
4-
import Transition, { ENTERING, ENTERED, EXITING } from 'react-transition-group/Transition';
4+
import Transition from 'react-transition-group/Transition';
55
import { mapToCssModules, TransitionTimeouts } from './utils';
66
import CarouselCaption from './CarouselCaption';
77

@@ -70,11 +70,11 @@ class CarouselItem extends React.Component {
7070
>
7171
{(status) => {
7272
const { direction } = this.context;
73-
const isActive = (status === ENTERED) || (status === EXITING);
74-
const directionClassName = (status === ENTERING || status === EXITING) &&
73+
const isActive = (status === 'entered') || (status === 'exiting');
74+
const directionClassName = (status === 'entering' || status === 'exiting') &&
7575
this.state.startAnimation &&
7676
(direction === 'right' ? 'carousel-item-left' : 'carousel-item-right');
77-
const orderClassName = (status === ENTERING) &&
77+
const orderClassName = (status === 'entering') &&
7878
(direction === 'right' ? 'carousel-item-next' : 'carousel-item-prev');
7979
const itemClasses = mapToCssModules(classNames(
8080
'carousel-item',

‎src/Collapse.js‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import React, { Component } from 'react';
22
import PropTypes from 'prop-types';
33
import classNames from 'classnames';
4-
import Transition, { EXITED, ENTERING, ENTERED, EXITING } from 'react-transition-group/Transition';
5-
import { mapToCssModules, omit, TransitionTimeouts } from './utils';
4+
import Transition from 'react-transition-group/Transition';
5+
import { mapToCssModules, omit, pick, TransitionTimeouts, TransitionPropTypeKeys } from './utils';
66

77
const propTypes = {
88
...Transition.propTypes,
@@ -27,17 +27,15 @@ const defaultProps = {
2727
timeout: TransitionTimeouts.Collapse,
2828
};
2929

30+
const transitionStatusToClassHash = {
31+
entering: 'collapsing',
32+
entered: 'collapse show',
33+
exiting: 'collapsing',
34+
exited: 'collapse',
35+
};
36+
3037
function getTransitionClass(status) {
31-
if (status === ENTERING) {
32-
return 'collapsing';
33-
} else if (status === ENTERED) {
34-
return 'collapse show';
35-
} else if (status === EXITING) {
36-
return 'collapsing';
37-
} else if (status === EXITED) {
38-
return 'collapse';
39-
}
40-
return 'collapse';
38+
return transitionStatusToClassHash[status] || 'collapse';
4139
}
4240

4341
function getHeight(node) {
@@ -92,11 +90,25 @@ class Collapse extends Component {
9290
navbar,
9391
cssModule,
9492
children,
95-
...transitionProps
93+
...otherProps
9694
} = this.props;
97-
const otherProps = omit(transitionProps, Object.keys(propTypes));
95+
9896
const { height } = this.state;
9997

98+
// In NODE_ENV=production the Transition.propTypes are wrapped which results in an
99+
// empty object "{}". This is the result of the `react-transition-group` babel
100+
// configuration settings. Therefore, to ensure that production builds work without
101+
// error, we can either explicitly define keys or use the Transition.defaultProps.
102+
// Using the Transition.defaultProps excludes any required props. Thus, the best
103+
// solution is to explicitly define required props in our utilities and reference these.
104+
// This also gives us more flexibility in the future to remove the prop-types
105+
// dependency in distribution builds (Similar to how `react-transition-group` does).
106+
// Note: Without omitting the `react-transition-group` props, the resulting child
107+
// Tag component would inherit the Transition properties as attributes for the HTML
108+
// element which results in errors/warnings for non-valid attributes.
109+
const transitionProps = pick(otherProps, TransitionPropTypeKeys);
110+
const childProps = omit(otherProps, TransitionPropTypeKeys);
111+
100112
return (
101113
<Transition
102114
{...transitionProps}
@@ -117,8 +129,8 @@ class Collapse extends Component {
117129
const style = height === null ? null : { height };
118130
return (
119131
<Tag
120-
{...otherProps}
121-
style={{ ...otherProps.style, ...style }}
132+
{...childProps}
133+
style={{ ...childProps.style, ...style }}
122134
className={classes}
123135
>
124136
{children}

‎src/Fade.js‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import React from 'react';
22
import PropTypes from 'prop-types';
33
import classNames from 'classnames';
4-
import Transition, { ENTERING, ENTERED } from 'react-transition-group/Transition';
5-
import { mapToCssModules, omit, TransitionTimeouts } from './utils';
4+
import Transition from 'react-transition-group/Transition';
5+
import { mapToCssModules, omit, pick, TransitionPropTypeKeys, TransitionTimeouts } from './utils';
66

77
const propTypes = {
88
...Transition.propTypes,
@@ -37,21 +37,34 @@ function Fade(props) {
3737
className,
3838
cssModule,
3939
children,
40-
...transitionProps
40+
...otherProps
4141
} = props;
42-
const otherProps = omit(transitionProps, Object.keys(propTypes));
42+
43+
// In NODE_ENV=production the Transition.propTypes are wrapped which results in an
44+
// empty object "{}". This is the result of the `react-transition-group` babel
45+
// configuration settings. Therefore, to ensure that production builds work without
46+
// error, we can either explicitly define keys or use the Transition.defaultProps.
47+
// Using the Transition.defaultProps excludes any required props. Thus, the best
48+
// solution is to explicitly define required props in our utilities and reference these.
49+
// This also gives us more flexibility in the future to remove the prop-types
50+
// dependency in distribution builds (Similar to how `react-transition-group` does).
51+
// Note: Without omitting the `react-transition-group` props, the resulting child
52+
// Tag component would inherit the Transition properties as attributes for the HTML
53+
// element which results in errors/warnings for non-valid attributes.
54+
const transitionProps = pick(otherProps, TransitionPropTypeKeys);
55+
const childProps = omit(otherProps, TransitionPropTypeKeys);
4356

4457
return (
4558
<Transition {...transitionProps}>
4659
{(status) => {
47-
const isActive = status === ENTERING || status === ENTERED;
60+
const isActive = status === 'entering' || status === 'entered';
4861
const classes = mapToCssModules(classNames(
4962
className,
5063
baseClass,
51-
isActive ? baseClassActive : false
64+
isActive && baseClassActive
5265
), cssModule);
5366
return (
54-
<Tag className={classes} {...otherProps}>
67+
<Tag className={classes} {...childProps}>
5568
{children}
5669
</Tag>
5770
);

‎src/utils.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,23 @@ export function omit(obj, omitKeys) {
6262
return result;
6363
}
6464

65+
/**
66+
* Returns a filtered copy of an object with only the specified keys.
67+
*/
68+
export function pick(obj, keys) {
69+
const pickKeys = Array.isArray(keys) ? keys : [keys];
70+
let length = pickKeys.length;
71+
let key;
72+
const result = {};
73+
74+
while (length > 0) {
75+
length -= 1;
76+
key = pickKeys[length];
77+
result[key] = obj[key];
78+
}
79+
return result;
80+
}
81+
6582
let warned = {};
6683

6784
export function warnOnce(message) {
@@ -120,6 +137,25 @@ export const TransitionTimeouts = {
120137
Carousel: 600, // $carousel-transition
121138
};
122139

140+
// Duplicated Transition.propType keys to ensure that Reactstrap builds
141+
// for distribution properly exclude these keys for nested child HTML attributes
142+
// since `react-transition-group` removes propTypes in production builds.
143+
export const TransitionPropTypeKeys = [
144+
'in',
145+
'mountOnEnter',
146+
'unmountOnExit',
147+
'appear',
148+
'enter',
149+
'exit',
150+
'timeout',
151+
'onEnter',
152+
'onEntering',
153+
'onEntered',
154+
'onExit',
155+
'onExiting',
156+
'onExited',
157+
];
158+
123159
export const keyCodes = {
124160
esc: 27,
125161
space: 32,

0 commit comments

Comments
 (0)