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.

Sign up for GitHub

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

Added react-dom/test-utils bundle #9414

Closed
wants to merge 3 commits into from
Closed

Added react-dom/test-utils bundle #9414

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70d5c46
Added react-dom/test-utils target
Apr 13, 2017
aa88a15
Removed level of indirection for checkPropTypes
Apr 13, 2017
681a32e
Updated Rollup bundle for test-utils
Apr 13, 2017
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
37 changes: 29 additions & 8 deletions scripts/rollup/bundles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ const bundles = [
'src/renderers/shared/**/*.js',
'src/test/**/*.js', // ReactTestUtils is currently very coupled to DOM.

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down Expand Up @@ -120,7 +119,33 @@ const bundles = [
'src/renderers/shared/**/*.js',
'src/test/**/*.js', // ReactTestUtils is currently very coupled to DOM.

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
},
{
babelOpts: babelOptsReact,
bundleTypes: [NODE_DEV],
config: {
destDir: 'build/',
globals: {
react: 'React',
},
moduleName: 'ReactTestUtils',
sourceMap: false,
},
entry: 'src/test/ReactTestUtils.js',
externals: ['prop-types', 'prop-types/checkPropTypes', 'react'],
hasteName: 'ReactTestUtils',
isRenderer: false,
label: 'react-test-utils',
manglePropertiesOnProd: false,
name: 'react-test-utils',
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you run build, you will see that there’s a single react-test-utils.development.js left in the /build/ folder while everything else is neatly put into folders. This is probably because name corresponds to npm name, but there is no top-level /packages/react-test-utils/ so it doesn’t understand where to take package.json from, and skips it. (I admit this behavior is confusing.)

The rule of thumb is that nothing should end up in top-level /build/ folder, and that /build/packages/ should be all publish-able “as is” after build. I don’t think this is currently publish-able as is because test utils don’t actually make their way into the react-dom package.

You probably want to put 'react-dom/test-utils' in this field instead. This will ensure it gets copied into /build/packages/react-dom/ instead. You can see we are using the same trick for react-dom/server.

However, I’m not sure how it will end up on npm. Don’t we need to add a test-utils.js file to /packages/react-dom/ which points to the entry point, similarly to how we have server.js in /packages/react-dom/? And we’d probably want to add more entries to "files" in /packages/react-dom/package.json so that the new files actually end up in the package. Otherwise this PR does not really do anything to make it appear in the react-dom package.

I am slightly concerned about www. I would like to avoid deviating too much between how ReactTestUtils works in open source and in www. Currently, ReactTestUtils is a shim (/scripts/rollup/shims/facebook-www/) that re-exports a hidden value from ReactDOM. If we’re separating them in open source, can we also separate them in www?

The way I see it, we could do it by removing ReactTestUtils from the secret exports of ReactDOMFBEntry.js and ReactDOMFiberFBEntry.js, removing the ReactTestUtils.js shim from /scripts/rollup/shims/facebook-www/, and instead adding a FB_DEV target to this bundle. We would need to make sure it imports require('react-dom') as an external so that it doesn’t duplicate ReactDOM inside.

I have a similar concern about duplication in the open source build too (which this PR implements). If you build it and look inside the bundle, it completely duplicates ReactDOM inside. However this means that components using findDOMNode will import them from react-dom that is different from the one react-dom/test-utils is using. If you look at 15.5, react-dom/test-utils intentionally shared the implementation with react-dom itself because otherwise it can’t function correctly. So I think the need to declare react-dom as an external affects not only FB bundle, but open source bundle as well.

Does this make sense?

paths: [
'src/renderers/dom/**/*.js',
'src/renderers/shared/**/*.js',
'src/test/**/*.js', // ReactTestUtils is currently very coupled to DOM.

'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down Expand Up @@ -151,7 +176,6 @@ const bundles = [
'src/renderers/dom/**/*.js',
'src/renderers/shared/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down Expand Up @@ -190,7 +214,6 @@ const bundles = [
'src/renderers/art/**/*.js',
'src/renderers/shared/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down Expand Up @@ -226,7 +249,6 @@ const bundles = [
'src/renderers/art/**/*.js',
'src/renderers/shared/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down Expand Up @@ -328,7 +350,6 @@ const bundles = [
'src/renderers/shared/**/*.js',
'src/renderers/testing/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand All @@ -355,12 +376,13 @@ const bundles = [
'src/renderers/shared/**/*.js',
'src/renderers/testing/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
},

// TODO (bvaughn) Add shallow renderer target

/******* React Noop Renderer (used only for fixtures/fiber-debugger) *******/
{
babelOpts: babelOptsReact,
Expand All @@ -383,7 +405,6 @@ const bundles = [
'src/renderers/noop/**/*.js',
'src/renderers/shared/**/*.js',

'src/isomorphic/classic/types/checkPropTypes.js',
'src/ReactVersion.js',
'src/shared/**/*.js',
],
Expand Down
90 changes: 47 additions & 43 deletions scripts/rollup/results.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,113 +1,113 @@
{
"branch": "master",
"branch": "react-dom-test-utils",
"bundleSizes": {
"react.development.js (UMD_DEV)": {
"size": 121454,
"gzip": 30515
"size": 121412,
"gzip": 30500
},
"react.production.min.js (UMD_PROD)": {
"size": 15685,
"gzip": 5765
"size": 15679,
"gzip": 5761
},
"react-dom.development.js (UMD_DEV)": {
"size": 583190,
"gzip": 134534
"size": 583148,
"gzip": 134526
},
"react-dom.production.min.js (UMD_PROD)": {
"size": 120740,
"gzip": 38094
},
"react-dom-server.development.js (UMD_DEV)": {
"size": 495558,
"gzip": 119685
"size": 495516,
"gzip": 119682
},
"react-dom-server.production.min.js (UMD_PROD)": {
"size": 107033,
"gzip": 33273
},
"react-art.development.js (UMD_DEV)": {
"size": 342608,
"gzip": 76782
"size": 342568,
"gzip": 76773
},
"react-art.production.min.js (UMD_PROD)": {
"size": 95013,
"gzip": 28991
},
"react.development.js (NODE_DEV)": {
"size": 70266,
"gzip": 17594
"size": 70222,
"gzip": 17579
},
"react.production.min.js (NODE_PROD)": {
"size": 9226,
"gzip": 3628
"size": 9220,
"gzip": 3621
},
"React-dev.js (FB_DEV)": {
"size": 72123,
"gzip": 18231
"size": 72079,
"gzip": 18217
},
"React-prod.js (FB_PROD)": {
"size": 36643,
"gzip": 9256
"size": 36606,
"gzip": 9248
},
"ReactDOMStack-dev.js (FB_DEV)": {
"size": 522763,
"gzip": 124727
"size": 522721,
"gzip": 124723
},
"ReactDOMStack-prod.js (FB_PROD)": {
"size": 352776,
"gzip": 84675
},
"react-dom.development.js (NODE_DEV)": {
"size": 542188,
"gzip": 125158
"size": 542144,
"gzip": 125150
},
"react-dom.production.min.js (NODE_PROD)": {
"size": 116925,
"gzip": 36732
},
"ReactDOMFiber-dev.js (FB_DEV)": {
"size": 797235,
"gzip": 184122
"size": 797189,
"gzip": 184111
},
"ReactDOMFiber-prod.js (FB_PROD)": {
"size": 407613,
"gzip": 93586
},
"react-dom-server.development.js (NODE_DEV)": {
"size": 445589,
"gzip": 107597
"size": 445547,
"gzip": 107594
},
"react-dom-server.production.min.js (NODE_PROD)": {
"size": 101411,
"gzip": 31292
},
"ReactDOMServerStack-dev.js (FB_DEV)": {
"size": 444281,
"gzip": 107443
"size": 444239,
"gzip": 107440
},
"ReactDOMServerStack-prod.js (FB_PROD)": {
"size": 334166,
"gzip": 80444
},
"ReactARTStack-dev.js (FB_DEV)": {
"size": 142986,
"gzip": 32714
"size": 142944,
"gzip": 32705
},
"ReactARTStack-prod.js (FB_PROD)": {
"size": 101143,
"gzip": 22993
},
"react-art.development.js (NODE_DEV)": {
"size": 265052,
"gzip": 56927
"size": 265008,
"gzip": 56923
},
"react-art.production.min.js (NODE_PROD)": {
"size": 56628,
"gzip": 17152
},
"ReactARTFiber-dev.js (FB_DEV)": {
"size": 264230,
"gzip": 56736
"size": 264186,
"gzip": 56732
},
"ReactARTFiber-prod.js (FB_PROD)": {
"size": 205336,
Expand All @@ -122,20 +122,24 @@
"gzip": 84001
},
"ReactTestRendererFiber-dev.js (FB_DEV)": {
"size": 262139,
"gzip": 55704
"size": 262095,
"gzip": 55698
},
"ReactTestRendererStack-dev.js (FB_DEV)": {
"size": 151521,
"gzip": 34765
"size": 151479,
"gzip": 34749
},
"react-noop-renderer.development.js (NODE_DEV)": {
"size": 254136,
"gzip": 53682
"size": 254092,
"gzip": 53674
},
"react-test-renderer.development.js (NODE_DEV)": {
"size": 262970,
"gzip": 55891
"size": 262926,
"gzip": 55887
},
"react-test-utils.development.js (NODE_DEV)": {
"size": 510240,
"gzip": 122093
}
}
}
2 changes: 1 addition & 1 deletion src/isomorphic/React.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var ReactPropTypes = require('ReactPropTypes');
var ReactVersion = require('ReactVersion');

var onlyChild = require('onlyChild');
var checkPropTypes = require('checkPropTypes');
var checkPropTypes = require('prop-types/checkPropTypes');
var createReactClass = require('createClass');

var createElement = ReactElement.createElement;
Expand Down
2 changes: 1 addition & 1 deletion src/isomorphic/classic/element/ReactElementValidator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var getComponentName = require('getComponentName');
var getIteratorFn = require('getIteratorFn');

if (__DEV__) {
var checkPropTypes = require('checkPropTypes');
var checkPropTypes = require('prop-types/checkPropTypes');
var warning = require('fbjs/lib/warning');
var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame');
var {
Expand Down
2 changes: 1 addition & 1 deletion src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var MyComponent;

function resetWarningCache() {
jest.resetModules();
checkPropTypes = require('checkPropTypes');
checkPropTypes = require('prop-types/checkPropTypes');
}

function getPropTypeWarningMessage(propTypes, object, componentName) {
Expand Down
14 changes: 0 additions & 14 deletions src/isomorphic/classic/types/checkPropTypes.js
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions src/node_modules/react/lib/checkPropTypes.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import type {Fiber} from 'ReactFiber';
import type {StackCursor} from 'ReactFiberStack';

var checkPropTypes = require('checkPropTypes');
var checkPropTypes = require('prop-types/checkPropTypes');
var emptyObject = require('fbjs/lib/emptyObject');
var getComponentName = require('getComponentName');
var invariant = require('fbjs/lib/invariant');
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (__DEV__) {
var warningAboutMissingGetChildContext = {};
}

var checkPropTypes = require('checkPropTypes');
var checkPropTypes = require('prop-types/checkPropTypes');
var emptyObject = require('fbjs/lib/emptyObject');
var invariant = require('fbjs/lib/invariant');
var shallowEqual = require('fbjs/lib/shallowEqual');
Expand Down