Skip to content
This repository was archived by the owner on Mar 31, 2021. It is now read-only.

React 16 + Ref Support #106

Merged
Robdel12 merged 6 commits into from
Nov 6, 2018
Merged

React 16 + Ref Support #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afe1508
Mark react 16 as a valid peer dependency.
Oct 17, 2018
a828f7c
Update tests to run against React 16 and verify everything works.
Oct 17, 2018
2d5979f
Add integration test case for refs that reproduces issue seen with
Oct 18, 2018
d7ba894
Extract react and react-dom into separate bundle to avoid loading rea…
Oct 18, 2018
38130be
Update test to correctly expect six snapshots.
Oct 18, 2018
9f1ab00
Check in yarn integrity SHAs in `yarn.lock`
Robdel12 Nov 1, 2018
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
2 changes: 1 addition & 1 deletion integration-tests/react-percy/__tests__/react-percy-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ beforeEach(() => {
});

const expectPercyToHaveRunSnapshots = () => {
const expectedSnapshots = 3;
const expectedSnapshots = 6;

expect(percy.createBuild).toHaveBeenCalledTimes(1);
expect(percy.uploadResources).toHaveBeenCalledTimes(1);
Expand Down
6 changes: 3 additions & 3 deletions integration-tests/react-percy/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
"percy": "react-percy"
},
"dependencies": {
"prop-types": "^15.5.10",
"react": "^15.6.1",
"react-dom": "^15.6.1"
"prop-types": "^15.6.2",
"react": "^16.5.2",
"react-dom": "^16.5.2"
Copy link
Contributor Author

@thomashuston thomashuston Oct 18, 2018

Choose a reason for hiding this comment

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

A while back we discussed adding better integration test coverage of multiple React versions. Might be worth a separate PR that adds coverage for both React 15 and 16 and both create-react-app 1 and 2.

},
"devDependencies": {
"@percy/react": "^0.4.6",
Expand Down
41 changes: 41 additions & 0 deletions integration-tests/react-percy/src/Refs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import PropTypes from 'prop-types';
import React from 'react';

export default class Refs extends React.Component {
static propTypes = {
children: PropTypes.string.isRequired,
type: PropTypes.oneOf(['string', 'function', 'createRef']).isRequired,
};

ref = React.createRef();

render() {
switch (this.props.type) {
case 'string':
return (
// eslint-disable-next-line react/no-string-refs
<div ref="test">
{this.props.children}
</div>
);

case 'function':
return (
<div
ref={el => {
this.el = el;
}}
>
{this.props.children}
</div>
);

case 'createRef':
return (
<div ref={this.ref}>
{this.props.children}
</div>
);
}
}
}
16 changes: 16 additions & 0 deletions integration-tests/react-percy/src/__percy__/Refs.percy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react';
import Refs from '../Refs';

suite('Refs', () => {
percySnapshot('components with string refs work', () => {
return <Refs type="string">This is some text</Refs>;
});

percySnapshot('components with function refs work', () => {
return <Refs type="function">This is some text</Refs>;
});

percySnapshot('components using createRef work', () => {
return <Refs type="createRef">This is some text</Refs>;
});
});
4 changes: 4 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"lerna": "^2.0.0",
"lint-staged": "^3.3.1",
"prettier": "^1.5.3",
"raf": "^3.4.0",
"rimraf": "^2.5.4",
"tar.gz": "^1.0.5",
"vitruvius": "^3.0.0"
Expand All @@ -44,6 +45,9 @@
"<rootDir>/integration-tests/",
"<rootDir>/packages/"
],
"setupFiles": [
"raf/polyfill"
Copy link
Contributor Author

@thomashuston thomashuston Oct 18, 2018

Choose a reason for hiding this comment

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

React 16 requires a requestAnimationFrame polyfill for tests to run properly.

Copy link
Contributor

@Robdel12 Robdel12 Oct 31, 2018

Choose a reason for hiding this comment

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

Looks like Jest 21.3.x included the polyfill by default. Maybe we can bump the version of Jest here (or a separate PR?) jestjs/jest#4568 (comment)

Copy link
Contributor Author

@thomashuston thomashuston Oct 31, 2018

Choose a reason for hiding this comment

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

Sure thing, I’ll toss up a separate PR for that next week. I’m off on vacation till then :)

],
"testPathIgnorePatterns": [
"/node_modules/",
"<rootDir>/integration-tests/create-react-app",
Expand Down
4 changes: 3 additions & 1 deletion packages/react-percy-ci/src/__tests__/__snapshots__/getHTML-tests.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ exports[`generates HTML with all stylesheets and the three entry scripts 1`] = `
</head>
<body>
<div id=\\"percy-root\\"></div>
<script type=\\"text/javascript\\" src=\\"/static/percy_framework.js\\"></script>
<script type=\\"text/javascript\\" src=\\"/static/percy_runtime.js\\"></script>
<script type=\\"text/javascript\\" src=\\"/static/percy_vendor.js\\"></script>
<script type=\\"text/javascript\\" src=\\"/static/percy_framework.js\\"></script>
<script type=\\"text/javascript\\" src=\\"/static/percy_snapshots.js\\"></script>
<script type=\\"text/javascript\\" src=\\"/static/percy_render.js\\"></script>
</body>
Expand Down
2 changes: 2 additions & 0 deletions packages/react-percy-ci/src/__tests__/getHTML-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ it('generates HTML with all stylesheets and the three entry scripts', () => {
'/static/styles2.css': 'styles 2 CSS',
[`/static/${EntryNames.framework}.js`]: 'framework JS',
[`/static/${EntryNames.render}.js`]: 'render JS',
[`/static/${EntryNames.runtime}.js`]: 'runtime JS',
[`/static/${EntryNames.snapshots}.js`]: 'snapshots JS',
[`/static/${EntryNames.vendor}.js`]: 'vendor JS',
'/static/other.js': 'other JS',
'/static/foo.jpg': 'foo JPG',
};
Expand Down
2 changes: 2 additions & 0 deletions packages/react-percy-ci/src/getHTML.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export default function getHTML(assets) {
const stylesheets = getStylesheets(assets);

const scripts = [
findEntryPath(assets, EntryNames.runtime),
findEntryPath(assets, EntryNames.vendor),
findEntryPath(assets, EntryNames.framework),
findEntryPath(assets, EntryNames.snapshots),
findEntryPath(assets, EntryNames.render),
Expand Down
10 changes: 8 additions & 2 deletions packages/react-percy-ci/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ export default async function run(percyConfig, percyToken) {
const assets = await compile(percyConfig);

const environment = new Environment();
const runtimeEntry = findEntryPath(assets, EntryNames.runtime);
Copy link
Contributor Author

@thomashuston thomashuston Oct 18, 2018

Choose a reason for hiding this comment

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

The runtime entry contains a manifest webpack uses to make sure imports work correctly across entries now that we're splitting some dependencies out into the vendor bundle. The vendor bundle contains react and react-dom, so that they're not included in multiple scripts.

const vendorEntry = findEntryPath(assets, EntryNames.vendor);
const snapshotsEntry = findEntryPath(assets, EntryNames.snapshots);
debug('executing snapshots script');
environment.runScript(assets[snapshotsEntry]);
debug('executing scripts');
environment.runScript(`
${assets[runtimeEntry]}
${assets[vendorEntry]}
${assets[snapshotsEntry]}
`);

debug('getting snapshots');
const snapshots = environment.getSnapshotDefinitions();
Expand Down
10 changes: 5 additions & 5 deletions packages/react-percy-snapshot-render/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
"lib"
],
"peerDependencies": {
"react": "^15.4.0",
"react-dom": "^15.4.0"
"react": "^15.4.0 || ^16.0.0",
"react-dom": "^15.4.0 || ^16.0.0"
Copy link
Contributor Author

@thomashuston thomashuston Oct 18, 2018

Choose a reason for hiding this comment

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

This allows users to have either React 15 or React 16 in their projects without getting any peer dependency warnings.

},
"dependencies": {
"babel-runtime": "^6.26.0",
"redbox-react": "^1.5.0"
"redbox-react": "^1.6.0"
},
"devDependencies": {
"react": "^15.6.1",
"react-dom": "^15.6.1"
"react": "^16.5.2",
"react-dom": "^16.5.2"
},
"publishConfig": {
"access": "public"
Expand Down
4 changes: 2 additions & 2 deletions packages/react-percy-snapshot-render/src/__tests__/__snapshots__/render-tests.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`renders error message into the DOM element when rendering fails 1`] = `"<div data-reactroot=\\"\\">Cannot read property 'format' of undefined</div>"`;
exports[`renders error message into the DOM element when rendering fails 1`] = `"<div>Cannot read property 'format' of undefined</div>"`;

exports[`renders the specified React markup into the DOM element 1`] = `"<div data-reactroot=\\"\\">some markup</div>"`;
exports[`renders the specified React markup into the DOM element 1`] = `"<div>some markup</div>"`;
3 changes: 3 additions & 0 deletions packages/react-percy-snapshot-render/src/__tests__/render-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ jest.mock('redbox-react', () => ({ error }) =>
);
/* eslint-enable react/display-name, react/prop-types */

// eslint-disable-next-line no-console
console.error = jest.fn();
Copy link
Contributor Author

@thomashuston thomashuston Oct 18, 2018

Choose a reason for hiding this comment

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

This test purposefully causes an error to get thrown so we can verify it's getting rendered. React 16 has a new mechanism for catching errors and prints a console error here, but it's not really useful in the context of this test.


let el;

beforeEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-percy-test-framework/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"promise-each": "^2.2.0"
},
"devDependencies": {
"react": "^15.4.2"
"react": "^16.5.2"
},
"publishConfig": {
"access": "public"
Expand Down
3 changes: 3 additions & 0 deletions packages/react-percy-webpack/src/compile/__tests__/createCompiler-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const mockCompiler = () => new WebpackCompiler();
jest.mock('webpack', () => {
const webpack = jest.fn(() => mockCompiler());
webpack.DefinePlugin = class MockDefinePlugin {};
webpack.optimize = {
CommonsChunkPlugin: class CommonsChunkPlugin {},
};
return webpack;
});

Expand Down
8 changes: 8 additions & 0 deletions packages/react-percy-webpack/src/compile/createCompiler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import babelConfig from './babelConfig';
import CaseSensitivePathsPlugin from 'case-sensitive-paths-webpack-plugin';
import { EntryNames } from '../entry/constants';
import getEntry from '../entry';
import MemoryOutputPlugin from './MemoryOutputPlugin';
import merge from 'webpack-merge';
Expand Down Expand Up @@ -47,6 +48,13 @@ export default function createCompiler(percyConfig) {
},
}),
new CaseSensitivePathsPlugin(),
new webpack.optimize.CommonsChunkPlugin({
name: EntryNames.vendor,
minChunks: Infinity,
}),
new webpack.optimize.CommonsChunkPlugin({
name: EntryNames.runtime,
}),
percyConfig.debug ? undefined : new MemoryOutputPlugin('/static/'),
].filter(plugin => plugin !== undefined),
resolve: {
Expand Down
17 changes: 17 additions & 0 deletions packages/react-percy-webpack/src/entry/__tests__/getEntry-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,20 @@ it('returns snapshots entry containing include files and snapshots entry file gi
}),
);
});

it('returns vendor entry containing react and react-dom', () => {
const percyConfig = {
includeFiles: [],
renderer: '@percy/react-percy-snapshot-render',
rootDir: '/foo',
snapshotPatterns: ['**/__percy__/*.js', '**/*.percy.js'],
};

const entry = getEntry(percyConfig);

expect(entry).toEqual(
expect.objectContaining({
[EntryNames.vendor]: ['react', 'react-dom'],
}),
);
});
2 changes: 2 additions & 0 deletions packages/react-percy-webpack/src/entry/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
export const EntryNames = {
framework: 'percy_framework',
render: 'percy_render',
runtime: 'percy_runtime',
snapshots: 'percy_snapshots',
vendor: 'percy_vendor',
};

export const GlobalVariables = {
Expand Down
1 change: 1 addition & 0 deletions packages/react-percy-webpack/src/entry/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ export default function getEntry(percyConfig) {
[EntryNames.framework]: frameworkEntryFile,
[EntryNames.render]: renderEntryFile,
[EntryNames.snapshots]: [...percyConfig.includeFiles, snapshotsEntryFile],
[EntryNames.vendor]: ['react', 'react-dom'],
};
}
Loading