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

Upgrading deps #446

Merged
merged 11 commits into from
Apr 17, 2018
22 changes: 11 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
{
"dist/react-beautiful-dnd.js": {
"bundled": 372683,
"minified": 142833,
"gzipped": 40327
"bundled": 399839,
"minified": 149239,
"gzipped": 42623
},
"dist/react-beautiful-dnd.min.js": {
"bundled": 334038,
"minified": 126662,
"gzipped": 35556
"bundled": 358155,
"minified": 132319,
"gzipped": 37652
},
"dist/react-beautiful-dnd.esm.js": {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

our esm builds are looking better for moving to invariant rather than throwing, but otherwise we took a huge hit!

Choose a reason for hiding this comment

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

What is the reason for using invariant over throw statements? Did you notice any effect on Flow's ability to narrow the types on things?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is the reason for using invariant:

 * The invariant message will be stripped in production, but the invariant
 * will remain to ensure logic does not differ in production.

Flow understands that invariant will throw if the condition is not met

"bundled": 176840,
"minified": 88989,
"gzipped": 22594,
"bundled": 176656,
"minified": 88917,
"gzipped": 22577,
"treeshaked": {
"rollup": 77631,
"webpack": 79229
"rollup": 77471,
"webpack": 79091
}
}
}
31 changes: 16 additions & 15 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,22 @@
},
"dependencies": {
"babel-runtime": "^6.26.0",
"invariant": "^2.2.4",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gone!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wrote my own. I will put it on npm soon.

"memoize-one": "^3.1.1",
"prop-types": "^15.6.0",
"raf-schd": "^2.1.1",
"react-motion": "^0.5.2",
"react-redux": "^5.0.6",
"redux": "^3.7.2",
"redux-thunk": "^2.2.0",
"reselect": "^3.0.1"
"reselect": "^3.0.1",
"tiny-invariant": "^0.0.3"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right now i have not figured out how to get our rollup build to tree shake the env conditional insight of this module. However, i am happy to solve that later

},
"devDependencies": {
"@atlaskit/css-reset": "^2.0.0",
"@storybook/react": "^3.2.16",
"@storybook/react": "^3.4.1",
"babel-cli": "^6.26.0",
"babel-eslint": "^8.2.1",
"babel-core": "^6.26.0",

Choose a reason for hiding this comment

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

No babel-core before this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it was being brought in by another dep. now we are being more explicit

"babel-eslint": "^8.2.3",
"babel-plugin-transform-class-properties": "^6.24.1",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.0",
"babel-plugin-transform-export-extensions": "^6.22.0",
Expand All @@ -67,33 +68,33 @@
"babel-preset-env": "^1.6.1",
"babel-preset-flow": "^6.23.0",
"babel-preset-react": "^6.24.1",
"cross-env": "^5.1.3",
"cross-env": "^5.1.4",
"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
"eslint": "^4.15.0",
"eslint": "^4.19.1",
"eslint-config-airbnb": "^16.1.0",
"eslint-plugin-flowtype": "^2.41.0",
"eslint-plugin-import": "^2.8.0",
"eslint-plugin-jest": "^21.6.1",
"eslint-plugin-flowtype": "^2.46.2",
"eslint-plugin-import": "^2.11.0",
"eslint-plugin-jest": "^21.15.0",
"eslint-plugin-jsx-a11y": "^6.0.3",
"eslint-plugin-react": "^7.5.1",
"flow-bin": "0.69.0",
"jest": "^22.0.5",
"puppeteer": "^1.2.0",
"eslint-plugin-react": "^7.7.0",
"flow-bin": "0.70.0",
"jest": "^22.4.3",
"puppeteer": "^1.3.0",
"raf-stub": "^2.0.2",
"react": "^16.3.1",
"react-dom": "^16.3.1",
"react-test-renderer": "^16.3.1",
"rimraf": "^2.6.2",
"rollup": "^0.56.2",
"rollup": "^0.58.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

could this be the cause?

"rollup-plugin-babel": "^3.0.3",
"rollup-plugin-commonjs": "^8.2.6",
"rollup-plugin-node-resolve": "^3.0.2",
"rollup-plugin-replace": "^2.0.0",
"rollup-plugin-size-snapshot": "^0.3.0",
"rollup-plugin-strip": "^1.1.1",
"rollup-plugin-uglify": "^2.0.1",
"styled-components": "^3.2.0"
"styled-components": "^3.2.5"
},
"peerDependencies": {
"react": "^16.3.1"
Expand Down
3 changes: 2 additions & 1 deletion rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ export default [
// Only deep dependency required is React
external: ['react'],
plugins: [
// Setting production env before running babel etc
replace({ 'process.env.NODE_ENV': JSON.stringify('production') }),
babel(getBabelOptions()),
resolve({ extensions }),
commonjs({ include: 'node_modules/**' }),
replace({ 'process.env.NODE_ENV': JSON.stringify('production') }),
strip({ debugger: true }),
sizeSnapshot({ updateSnapshot: !shouldCheckSnapshot }),
uglify(),
Expand Down
5 changes: 2 additions & 3 deletions src/state/auto-scroller/get-best-scrollable-droppable.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
import isPositionInFrame from '../visibility/is-position-in-frame';
import type {
Position,
Expand Down Expand Up @@ -36,9 +37,7 @@ const getScrollableDroppableOver = (
const maybe: ?DroppableDimension =
getScrollableDroppables(droppables)
.find((droppable: DroppableDimension): boolean => {
if (!droppable.viewport.closestScrollable) {
throw new Error('Invalid result');
}
invariant(droppable.viewport.closestScrollable, 'Invalid result');
return isPositionInFrame(droppable.viewport.closestScrollable.frame)(target);
});

Expand Down
5 changes: 3 additions & 2 deletions src/state/dimension.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,11 @@ export const scrollDroppable = (
clipped,
};

return ({
const result: DroppableDimension = {

Choose a reason for hiding this comment

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

Does flow complain without this cast?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope! it now works with exact types and spread

...droppable,
viewport,
}: any);
};
return result;
};

type GetDroppableArgs = {|
Expand Down
6 changes: 3 additions & 3 deletions src/state/move-cross-axis/get-best-cross-axis-droppable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @flow
import invariant from 'tiny-invariant';
import { closest } from '../position';
import isWithin from '../is-within';
import { getCorners } from '../spacing';
Expand Down Expand Up @@ -27,9 +28,8 @@ type GetBestDroppableArgs = {|
const getSafeClipped = (droppable: DroppableDimension): Area => {
const area: ?Area = droppable.viewport.clipped;

if (!area) {
throw new Error('cannot get clipped area from droppable');
}
invariant(area, 'Cannot get clipped area from droppable');

return area;
};

Expand Down
4 changes: 2 additions & 2 deletions src/state/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,13 @@ export default (state: State = clean('IDLE'), action: Action): State => {
return existing;
}

const newDrag: DragState = ({
const newDrag: DragState = {

Choose a reason for hiding this comment

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

Could clean up the Flow annotations by typing this IIFE like:

const drag = ((): ?DragState => {
  // ...
})();

...existing,
current: {
...existing.current,
hasCompletedFirstBulkPublish: true,
},
}: any);
};

return newDrag;
})();
Expand Down
4 changes: 2 additions & 2 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,15 @@ export type DropState = {|
result: ?DropResult,
|}

export type State = {
export type State = {|
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now an exact type

phase: Phase,
dimension: DimensionState,
// null if not dragging
drag: ?DragState,

// available when dropping or cancelling
drop: ?DropState,
};
|};

export type Action = ActionCreators;
export type Dispatch = ReduxDispatch<Action>;
Expand Down
5 changes: 2 additions & 3 deletions src/view/announcer/announcer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @flow
import invariant from 'tiny-invariant';
import type { Announce } from '../../types';
import type { Announcer } from './announcer-types';

Expand Down Expand Up @@ -65,9 +66,7 @@ export default (): Announcer => {
// hide the element visually
Object.assign(el.style, visuallyHidden);

if (!document.body) {
throw new Error('Cannot find the head to append a style to');
}
invariant(document.body, 'Cannot find the head to append a style to');

// add el tag to body
document.body.appendChild(el);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Component } from 'react';
import type { Node } from 'react';
import PropTypes from 'prop-types';
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
import getWindowScroll from '../window/get-window-scroll';
import { getDraggableDimension } from '../../state/dimension';
import { dimensionMarshalKey } from '../context-keys';
Expand Down Expand Up @@ -90,15 +91,11 @@ export default class DraggableDimensionPublisher extends Component<Props> {
getDimension = (): DraggableDimension => {
const targetRef: ?HTMLElement = this.props.getDraggableRef();

if (!targetRef) {
throw new Error('DraggableDimensionPublisher cannot calculate a dimension when not attached to the DOM');
}
invariant(targetRef, 'DraggableDimensionPublisher cannot calculate a dimension when not attached to the DOM');

const descriptor: ?DraggableDescriptor = this.publishedDescriptor;

if (!descriptor) {
throw new Error('Cannot get dimension for unpublished draggable');
}
invariant(descriptor, 'Cannot get dimension for unpublished draggable');

const tagName: string = targetRef.tagName.toLowerCase();
const style = window.getComputedStyle(targetRef);
Expand Down
6 changes: 2 additions & 4 deletions src/view/draggable/draggable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { Component, Fragment } from 'react';
import type { Node } from 'react';
import PropTypes from 'prop-types';
import memoizeOne from 'memoize-one';
import invariant from 'invariant';
import invariant from 'tiny-invariant';
import type {
Position,
DraggableDimension,
Expand Down Expand Up @@ -250,9 +250,7 @@ export default class Draggable extends Component<Props> {
return this.getNotDraggingStyle(movementStyle, shouldAnimateDisplacement);
}

if (!dimension) {
throw new Error('draggable dimension required for dragging');
}
invariant(dimension, 'draggable dimension required for dragging');

// Need to position element in original visual position. To do this
// we position it without
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Component } from 'react';
import type { Node } from 'react';
import PropTypes from 'prop-types';
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
import rafSchedule from 'raf-schd';
import getWindowScroll from '../window/get-window-scroll';
import getArea from '../../state/get-area';
Expand Down Expand Up @@ -239,20 +240,11 @@ export default class DroppableDimensionPublisher extends Component<Props> {
} = this.props;

const targetRef: ?HTMLElement = getDroppableRef();

if (!targetRef) {
throw new Error('DimensionPublisher cannot calculate a dimension when not attached to the DOM');
}

if (this.isWatchingScroll) {
throw new Error('Attempting to recapture Droppable dimension while already watching scroll on previous capture');
}

const descriptor: ?DroppableDescriptor = this.publishedDescriptor;

if (!descriptor) {
throw new Error('Cannot get dimension for unpublished droppable');
}
invariant(targetRef, 'DimensionPublisher cannot calculate a dimension when not attached to the DOM');
invariant(!this.isWatchingScroll, 'Attempting to recapture Droppable dimension while already watching scroll on previous capture');
invariant(descriptor, 'Cannot get dimension for unpublished droppable');

const style: Object = window.getComputedStyle(targetRef);

Expand Down
5 changes: 2 additions & 3 deletions src/view/style-marshal/style-marshal.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow
import memoizeOne from 'memoize-one';
import invariant from 'tiny-invariant';
import getStyles, { type Styles } from './get-styles';
import type { StyleMarshal } from './style-marshal-types';
import type {
Expand Down Expand Up @@ -60,9 +61,7 @@ export default () => {
el.setAttribute(prefix, context);
const head: ?HTMLElement = document.querySelector('head');

if (!head) {
throw new Error('Cannot find the head to append a style to');
}
invariant(head, 'Cannot find the head to append a style to');

// add style tag to head
head.appendChild(el);
Expand Down
5 changes: 4 additions & 1 deletion src/view/window/get-viewport.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @flow
import invariant from 'tiny-invariant';
import type { Position, Area, Viewport } from '../../types';
import getArea from '../../state/get-area';
import getWindowScroll from './get-window-scroll';
Expand All @@ -10,7 +11,9 @@ export default (): Viewport => {
const top: number = scroll.y;
const left: number = scroll.x;

const doc: HTMLElement = (document.documentElement : any);
const doc: ?HTMLElement = document.documentElement;

invariant(doc, 'Could not find document.documentElement');

// Using these values as they do not consider scrollbars
const width: number = doc.clientWidth;
Expand Down
3 changes: 0 additions & 3 deletions stories/src/multi-drag/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const reorderSingleDrag = ({
destination.index,
);

// $ExpectError - using spread
const updated: Entities = {
...entities,
columns: {
Expand Down Expand Up @@ -68,7 +67,6 @@ const reorderSingleDrag = ({
const newForeignTaskIds: Id[] = [...foreign.taskIds];
newForeignTaskIds.splice(destination.index, 0, taskId);

// $ExpectError - using spread
const updated: Entities = {
...entities,
columns: {
Expand Down Expand Up @@ -191,7 +189,6 @@ const reorderMultiDrag = ({
[final.id]: withNewTaskIds(final, withInserted),
};

// $ExpectError - using spread
const updated: Entities = {
...entities,
columns: withAddedTasks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ describe('lifting and the dimension marshal', () => {

// registering updated draggables

// $ExpectError - using spread
const postDragInHome1: DraggableDimension = {
...preset.inHome1,
descriptor: {
Expand All @@ -119,7 +118,6 @@ describe('lifting and the dimension marshal', () => {
index: 1,
},
};
// $ExpectError - using spread
const postDragInHome2: DraggableDimension = {
...preset.inHome2,
descriptor: {
Expand Down
Loading