Skip to content

using the wrong renderer's act() should warn #15398

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

Closed
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
8 changes: 7 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,10 @@ jobs:
path: ./node_modules.tgz

- store_artifacts:
path: ./scripts/error-codes/codes.json
path: ./build.tgz

- store_artifacts:
path: ./scripts/rollup/results.json

- store_artifacts:
path: ./scripts/error-codes/codes.json
67 changes: 59 additions & 8 deletions dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//
// `DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/react/pull/11865

const {markdown, danger} = require('danger');
const {markdown, danger, warn} = require('danger');
const fetch = require('node-fetch');

const {generateResultsArray} = require('./scripts/rollup/stats');
Expand Down Expand Up @@ -108,18 +108,69 @@ function git(args) {
// Use git locally to grab the commit which represents the place
// where the branches differ
const upstreamRepo = danger.github.pr.base.repo.full_name;
if (upstreamRepo !== 'facebook/react') {
// Exit unless we're running in the main repo
return;
}

const upstreamRef = danger.github.pr.base.ref;
await git(`remote add upstream https://github.com/${upstreamRepo}.git`);
await git(`remote add upstream https://github.com/facebook/react.git`);
await git('fetch upstream');
const mergeBaseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);
const baseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);

let resultsResponse = null;
try {
let baseCIBuildId = null;
const statusesResponse = await fetch(
`https://github.com/api/repos/facebook/react/commits/${baseCommit}/statuses`
);
const statuses = await statusesResponse.json();
for (let i = 0; i < statuses.length; i++) {
const status = statuses[i];
if (status.context === 'ci/circleci') {
if (status.state === 'success') {
baseCIBuildId = /\/facebook\/react\/([0-9]+)/.exec(
status.target_url
)[1];
break;
}
if (status.state === 'failure') {
warn(`Base commit is broken: ${baseCommit}`);
return;
}
}
}

if (baseCIBuildId === null) {
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
return;
}

const commitURL = sha =>
`http://react.zpao.com/builds/master/_commits/${sha}/results.json`;
const response = await fetch(commitURL(mergeBaseCommit));
const baseArtifactsInfoResponse = await fetch(
`https://circleci.com/api/v1.1/project/github/facebook/react/${baseCIBuildId}/artifacts`
);
const baseArtifactsInfo = await baseArtifactsInfoResponse.json();

for (let i = 0; i < baseArtifactsInfo.length; i++) {
const info = baseArtifactsInfo[i];
if (info.path === 'home/circleci/project/scripts/rollup/results.json') {
resultsResponse = await fetch(info.url);
break;
}
}
} catch (error) {
warn(`Failed to fetch build artifacts for base commit: ${baseCommit}`);
return;
}

if (resultsResponse === null) {
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
return;
}

// Take the JSON of the build response and
// make an array comparing the results for printing
const previousBuildResults = await response.json();
const previousBuildResults = await resultsResponse.json();
const results = generateResultsArray(
currentBuildResults,
previousBuildResults
Expand Down Expand Up @@ -212,7 +263,7 @@ function git(args) {
<details>
<summary>Details of bundled changes.</summary>

<p>Comparing: ${mergeBaseCommit}...${danger.github.pr.head.sha}</p>
<p>Comparing: ${baseCommit}...${danger.github.pr.head.sha}</p>


${allTables.join('\n')}
Expand Down
4 changes: 4 additions & 0 deletions fixtures/dom/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ public/react-dom-server.browser.development.js
public/react-dom-server.browser.production.min.js
public/react-dom-test-utils.development.js
public/react-dom-test-utils.production.min.js
public/react-test-renderer.development.js
public/react-test-renderer.production.min.js
public/scheduler.development.js
public/scheduler.production.min.js

# misc
.DS_Store
Expand Down
2 changes: 1 addition & 1 deletion fixtures/dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"scripts": {
"start": "react-scripts start",
"prestart": "cp ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/",
"prestart": "cp ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js ../../build/node_modules/react-test-renderer/umd/react-test-renderer.development.js ../../build/node_modules/react-test-renderer/umd/react-test-renderer.production.min.js ../../build/node_modules/scheduler/umd/scheduler.development.js ../../build/node_modules/scheduler/umd/scheduler.production.min.js public/",
"build": "react-scripts build && cp build/index.html build/200.html",
"test": "react-scripts test --env=jsdom",
"eject": "react-scripts eject"
Expand Down
77 changes: 47 additions & 30 deletions fixtures/dom/public/act-dom.html
Original file line number Diff line number Diff line change
@@ -1,41 +1,58 @@
<!DOCTYPE html>
<html>
<head>
<title>sanity test for ReactTestUtils.act</title>
</head>
<body>
this page tests whether act runs properly in a browser.
<br/>
your console should say "5"
<script src='react.development.js'></script>
<script src='react-dom.development.js'></script>
<script src='react-dom-test-utils.development.js'></script>
<script>
async function run(){
// from ReactTestUtilsAct-test.js
<head>
<title>sanity test for ReactTestUtils.act</title>
</head>
<body>
this page tests whether act runs properly in a browser.
<br />
your console should say "5", and a warning about not using the right act()
<script src="react.development.js"></script>
<script src="react-dom.development.js"></script>
<script src="react-dom-test-utils.development.js"></script>
<script src="scheduler.development.js"></script>
<script src="react-test-renderer.development.js"></script>
<script>
window.jest = {}; // to enable warnings
function App() {
let [state, setState] = React.useState(0);
async function ticker() {
await null;
setState(x => x + 1);
}
React.useEffect(
() => {
ticker();
},
[Math.min(state, 4)],
);
React.useEffect(() => {
ticker();
}, [Math.min(state, 4)]);
return state;
}
const el = document.createElement('div');
await ReactTestUtils.act(async () => {
ReactDOM.render(React.createElement(App), el);
});
// all 5 ticks present and accounted for
console.log(el.innerHTML);
}
run();

</script>
</body>

function sleep(period){
return new Promise(resolve => setTimeout(resolve, period))
}

async function testDOMAsynAct() {
// from ReactTestUtilsAct-test.js

const el = document.createElement("div");
await ReactTestUtils.act(async () => {
ReactDOM.render(React.createElement(App), el);
});
// all 5 ticks present and accounted for
console.log(el.innerHTML);
}

async function testMixRenderers() {
await ReactTestUtils.act(async () => {
ReactTestRenderer.create(React.createElement(App));
});
}

async function run() {
await testDOMAsynAct();
await testMixRenderers();
}

run();
</script>
</body>
</html>
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"flow-bin": "^0.72.0",
"glob": "^6.0.4",
"glob-stream": "^6.1.0",
"google-closure-compiler": "20190106.0.0",
"google-closure-compiler": "20190301.0.0",
"gzip-size": "^3.0.0",
"jasmine-check": "^1.0.0-rc.0",
"jest": "^23.1.0",
Expand Down Expand Up @@ -102,7 +102,6 @@
"test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
"test-persistent": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-persistent.js",
"test-fire": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-fire.js",
"test-new-scheduler": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-new-scheduler.js",
"test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js",
"test-fire-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source-fire.js",
"test-prod-build": "yarn test-build-prod",
Expand Down
43 changes: 0 additions & 43 deletions packages/events/EventTypes.js

This file was deleted.

43 changes: 35 additions & 8 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as Scheduler from 'scheduler';
import invariant from 'shared/invariant';

import {TYPES, EVENT_TYPES, childrenAsString} from './ReactARTInternals';
import type {ReactEventComponentInstance} from 'shared/ReactTypes';

// Intentionally not named imports because Rollup would
// use dynamic dispatch for CommonJS interop named imports.
Expand Down Expand Up @@ -439,19 +440,45 @@ export function unhideTextInstance(textInstance, text): void {
// Noop
}

export function handleEventComponent(
eventResponder: ReactEventResponder,
rootContainerInstance: Container,
internalInstanceHandle: Object,
export function mountEventComponent(
eventComponentInstance: ReactEventComponentInstance,
) {
throw new Error('Not yet implemented.');
}

export function updateEventComponent(
eventComponentInstance: ReactEventComponentInstance,
) {
// TODO: add handleEventComponent implementation
throw new Error('Not yet implemented.');
}

export function unmountEventComponent(
eventComponentInstance: ReactEventComponentInstance,
): void {
throw new Error('Not yet implemented.');
}

export function getEventTargetChildElement(
type: Symbol | number,
props: Props,
): null {
throw new Error('Not yet implemented.');
}

export function handleEventTarget(
type: Symbol | number,
props: Props,
parentInstance: Container,
rootContainerInstance: Container,
internalInstanceHandle: Object,
) {
// TODO: add handleEventTarget implementation
): boolean {
throw new Error('Not yet implemented.');
}

export function commitEventTarget(
type: Symbol | number,
props: Props,
instance: Instance,
parentInstance: Instance,
): void {
throw new Error('Not yet implemented.');
}
Loading