Skip to content
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

Simplify build #1035

Merged
merged 6 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
All notable changes to `dash` will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Changed
- [#1035](https://github.com/plotly/dash/pull/1035) Simplify our build process.

## [1.7.0] - 2019-11-27
### Added
- [#967](https://github.com/plotly/dash/pull/967) Add support for defining
Expand Down
20 changes: 0 additions & 20 deletions dash-renderer/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions dash-renderer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"react": "16.8.6",
"react-dom": "16.8.6",
"prop-types": "15.7.2",
"check-prop-types": "1.1.2",
"cookie": "^0.3.1",
"dependency-graph": "^0.5.0",
"radium": "^0.22.1",
Expand All @@ -32,7 +31,6 @@
"redux": "^3.4.0",
"redux-actions": "^0.9.1",
"redux-thunk": "^2.0.1",
"uniqid": "^5.0.3",
"viz.js": "1.8.0"
},
"devDependencies": {
Expand All @@ -59,7 +57,6 @@
"prettier-eslint": "^8.8.2",
"prettier-eslint-cli": "^4.7.1",
"prettier-stylelint": "^0.4.2",
"raw-loader": "^3.1.0",
"style-loader": "^1.0.0",
"webpack": "^4.39.3",
"webpack-cli": "^3.3.8",
Expand Down
2 changes: 1 addition & 1 deletion dash-renderer/src/TreeContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {notifyObservers, updateProps} from './actions';
import isSimpleComponent from './isSimpleComponent';
import {recordUiEdit} from './persistence';
import ComponentErrorBoundary from './components/error/ComponentErrorBoundary.react';
import checkPropTypes from 'check-prop-types';
import checkPropTypes from './checkPropTypes';

function validateComponent(componentDefinition) {
if (type(componentDefinition) === 'Array') {
Expand Down
88 changes: 88 additions & 0 deletions dash-renderer/src/checkPropTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copied out of prop-types and modified - inspired by check-prop-types, but
* simplified and tweaked to our needs: we don't need the NODE_ENV check,
* we report all errors, not just the first one, and we don't need the throwing
* variant `assertPropTypes`.
*/
import ReactPropTypesSecret from 'prop-types/lib/ReactPropTypesSecret';

/**
* Assert that the values match with the type specs.
*
* @param {object} typeSpecs Map of name to a ReactPropType
* @param {object} values Runtime values that need to be type-checked
* @param {string} location e.g. "prop", "context", "child context"
* @param {string} componentName Name of the component for error messages.
* @param {?Function} getStack Returns the component stack.
* @return {string} Any error messsage resulting from checking the types
*/
export default function checkPropTypes(
typeSpecs,
values,
location,
componentName,
getStack
) {
const errors = [];
for (const typeSpecName in typeSpecs) {
if (typeSpecs.hasOwnProperty(typeSpecName)) {
let error;
// Prop type validation may throw. In case they do, we don't want to
// fail the render phase where it didn't fail before. So we log it.
// After these have been cleaned up, we'll let them throw.
try {
// This is intentionally an invariant that gets caught. It's the same
// behavior as without this statement except with a better message.
if (typeof typeSpecs[typeSpecName] !== 'function') {
error = Error(
(componentName || 'React class') +
': ' +
location +
' type `' +
typeSpecName +
'` is invalid; ' +
'it must be a function, usually from the `prop-types` package, but received `' +
typeof typeSpecs[typeSpecName] +
'`.'
);
error.name = 'Invariant Violation';
} else {
error = typeSpecs[typeSpecName](
values,
typeSpecName,
componentName,
location,
null,
ReactPropTypesSecret
);
}
} catch (ex) {
error = ex;
}
if (error && !(error instanceof Error)) {
errors.push(
(componentName || 'React class') +
': type specification of ' +
location +
' `' +
typeSpecName +
'` is invalid; the type checker ' +
'function must return `null` or an `Error` but returned a ' +
typeof error +
'. ' +
'You may have forgotten to pass an argument to the type checker ' +
'creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and ' +
'shape all require an argument).'
);
}
if (error instanceof Error) {
var stack = (getStack && getStack()) || '';

errors.push(
'Failed ' + location + ' type: ' + error.message + stack
);
}
}
}
return errors.join('\n\n');
}
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the original file is under a BSD license, we should make sure that we either retain the original disclaimer as done in check-prop-type (https://github.com/ratehub/check-prop-types/blob/master/index.js#L7) or make sure the disclaimer for React is included explicitly in our bundled code -- which it wouldn't right now as we externalize React dependencies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current prop-types license is MIT (it was changed from BSD shortly after check-prop-types was published), as is check-prop-types - that presumably means this concern is moot, yes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moot as moot can be

Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ import {connect} from 'react-redux';
import {Component} from 'react';
import PropTypes from 'prop-types';
import Radium from 'radium';
import uniqid from 'uniqid';
import {onError, revert} from '../../actions';

class UnconnectedComponentErrorBoundary extends Component {
constructor(props) {
super(props);
this.state = {
myID: props.componentId,
myUID: uniqid(),
oldChildren: null,
hasError: false,
};
Expand All @@ -24,7 +22,6 @@ class UnconnectedComponentErrorBoundary extends Component {
const {dispatch} = this.props;
dispatch(
onError({
myUID: this.state.myUID,
myID: this.state.myID,
type: 'frontEnd',
error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import PropTypes from 'prop-types';
import '../Percy.css';
import {urlBase} from '../../../utils';

import werkzeugCss from '../werkzeug.css.txt';
import werkzeugCss from '../werkzeugcss';

class FrontEndError extends Component {
constructor(props) {
Expand Down Expand Up @@ -167,7 +167,6 @@ const ErrorContent = connect(state => ({base: urlBase(state.config)}))(

FrontEndError.propTypes = {
e: PropTypes.shape({
myUID: PropTypes.string,
timestamp: PropTypes.object,
error: errorPropTypes,
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// Werkzeug css included as a string, because we want to inject
// it into an iframe srcDoc

export default `
body {
margin: 0px;
margin-top: 10px;
Expand Down Expand Up @@ -102,3 +106,4 @@ div.debugger {
.debugger .traceback .source pre.line img {
display: none;
}
`;
2 changes: 0 additions & 2 deletions dash-renderer/src/persistence.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import {
type,
} from 'ramda';
import {createAction} from 'redux-actions';
import uniqid from 'uniqid';

import Registry from './registry';

Expand All @@ -81,7 +80,6 @@ function err(e) {
/* eslint-disable no-console */

return createAction('ON_ERROR')({
myUID: uniqid(),
myID: storePrefix,
type: 'frontEnd',
error,
Expand Down
16 changes: 2 additions & 14 deletions dash-renderer/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,18 @@ const defaults = {
rules: [
{
test: /\.js$/,
exclude: /node_modules\/(?!uniqid\/|check-prop-types\/)/,
exclude: /node_modules/,
use: {
loader: 'babel-loader',
},
},
{
test: /\.css$/,
use: [
{
loader: 'style-loader',
},
{
loader: 'css-loader',
},
],
use: ['style-loader', 'css-loader'],
},
{
test: /\.svg$/,
use: ['@svgr/webpack'],
},
{
test: /\.txt$/i,
use: 'raw-loader',
}
]
}
Expand All @@ -52,7 +41,6 @@ const rendererOptions = {
externals: {
react: 'React',
'react-dom': 'ReactDOM',
'plotly.js': 'Plotly',
'prop-types': 'PropTypes'
},
...defaults
Expand Down