Skip to content

Commit

Permalink
upgrade dev tools to flow 0.241 and fix sample app
Browse files Browse the repository at this point in the history
Summary:
Problem: the Relay repo is in flow `0.241` while the devtools are in `0.113.0` this means that if we use relay 17 in this project we will get a bunch of flow errors.

Additionally, all the nice features for react like component syntax and hook syntax are missing. To fix this I'm doing a least effort upgrade, fixing easy and repeatable patterns first and using `$FlowFixMe` for the rest.

Changes:
1. Explicit return types: used the codemods in this [note](https://medium.com/flow-type/local-type-inference-for-flow-aaa65d071347) and manually added the react component return types. For context, portal and fragments I just used FlowFixMe
2. Using objects as maps: Devtools uses objects as maps extensively, replaced those with FlowFixMe and we can move them to proper maps iteratively.
3. Bridge: Bridge had input and output types inferred, but they aren't used. Replaced with any.
4. Removed dead files: `Observable.js`, `isPromise.js` aren't required and had lots of errors, removing them was easier than upgrading.

It's a massive diff, but most changes are `FlowFixMe`'s.

To fix the sample app, I stringyfied and and parsed the store in the dev backend. We already do this for the chrome extension.

Reviewed By: tyao1

Differential Revision: D60495796

fbshipit-source-id: f5ddd5220015cb25adbf6c0fc3d84d8583118a39
  • Loading branch information
Andre Quispesaravia authored and facebook-github-bot committed Jul 31, 2024
1 parent d76af9a commit b784ca0
Show file tree
Hide file tree
Showing 68 changed files with 785 additions and 1,453 deletions.
13 changes: 6 additions & 7 deletions .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,19 @@ shells/dev/build/*
[include]

[libs]
/flow-typed/
./flow.js

[lints]

[options]
esproposal.class_instance_fields=enable
suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe
suppress_comment=\\(.\\|\n\\)*\\$FlowIssue
suppress_comment=\\(.\\|\n\\)*\\$FlowIgnore
module.name_mapper='^src' ->'<PROJECT_ROOT>/src'
esproposal.optional_chaining=enable
suppress_type=$FlowIssue
suppress_type=$FlowFixMe
suppress_type=$FlowFixMeProps
suppress_type=$FlowFixMeState
suppress_type=$FlowExpectedError

[strict]

[version]
^0.113.0
^0.241.0
6 changes: 3 additions & 3 deletions flow-typed/jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

type JestMockFn<TArguments: $ReadOnlyArray<*>, TReturn> = {
type JestMockFn<TArguments: $ReadOnlyArray<mixed>, TReturn> = {
(...args: TArguments): TReturn,
/**
* An object for introspecting mock calls
Expand Down Expand Up @@ -620,7 +620,7 @@ interface JestExpectType {
* Use .toBeInstanceOf(Class) to check that an object is an instance of a
* class.
*/
toBeInstanceOf(cls: Class<*>): void;
toBeInstanceOf(cls: Class<mixed>): void;
/**
* .toBeNull() is the same as .toBe(null) but the error messages are a bit
* nicer.
Expand Down Expand Up @@ -811,7 +811,7 @@ type JestObjectType = {
* Returns a new, unused mock function. Optionally takes a mock
* implementation.
*/
fn<TArguments: $ReadOnlyArray<*>, TReturn>(
fn<TArguments: $ReadOnlyArray<mixed>, TReturn>(
implementation?: (...args: TArguments) => TReturn
): JestMockFn<TArguments, TReturn>,
/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
"eslint-plugin-react-hooks": "^2.2.0",
"events": "^3.0.0",
"firefox-profile": "^1.0.2",
"flow-bin": "^0.113.0",
"flow-bin": "^0.241.0",
"fs-extra": "^3.0.1",
"graphql": "^16.9.0",
"jest": "^24.9.0",
Expand Down
11 changes: 3 additions & 8 deletions packages/relay-devtools-core/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ installHook(window);

const hook: DevToolsHook = window.__RELAY_DEVTOOLS_HOOK__;

function debug(methodName: string, ...args) {
function debug(methodName: string, ...args: [] | [any] | [string] | [Array<WallEvent>]) {
if (__DEBUG__) {
console.log(
`%c[core/backend] %c${methodName}`,
Expand All @@ -40,12 +40,7 @@ function debug(methodName: string, ...args) {
}

export function connectToDevTools(options: ?ConnectOptions) {
const {
host = 'localhost',
port = 8097,
websocket,
isAppActive = () => true,
} = options || {};
const {host = 'localhost', port = 8097, websocket, isAppActive = () => true} = options || {};

let retryTimeoutID: TimeoutID | null = null;

Expand Down Expand Up @@ -140,7 +135,7 @@ export function connectToDevTools(options: ?ConnectOptions) {
scheduleRetry();
}

function handleMessage(event) {
function handleMessage(event: any | MessageEvent) {
let data;
try {
if (typeof event.data === 'string') {
Expand Down
7 changes: 4 additions & 3 deletions packages/relay-devtools-core/src/launchEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export default function launchEditor(
args.push(filePath);
}

if (childProcess && isTerminalEditor(editor)) {
if (isTerminalEditor(editor) && childProcess) {
// There's an existing editor process already and it's attached
// to the terminal, so go kill it. Otherwise two separate editor
// instances attach to the stdin/stdout which gets confusing.
Expand All @@ -171,8 +171,9 @@ export default function launchEditor(
} else {
childProcess = spawn(editor, args, { stdio: 'inherit' });
}
childProcess.on('error', function() {});
childProcess.on('exit', function(errorCode) {
const cp = childProcess;
cp.on('error', function() {});
cp.on('exit', function(errorCode) {
childProcess = null;
});
}
11 changes: 6 additions & 5 deletions packages/relay-devtools/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const argv = require('minimist')(process.argv.slice(2));
const projectRoots = argv._;
const defaultThemeName = argv.theme;

let mainWindow = null;
let mainWindow: null | typeof BrowserWindow = null;

app.on('window-all-closed', function() {
app.quit();
Expand All @@ -31,26 +31,27 @@ app.on('ready', function() {
nodeIntegration: true,
},
});
const mw = mainWindow;

// and load the index.html of the app.
mainWindow.loadURL('file://' + __dirname + '/app.html'); // eslint-disable-line no-path-concat
mainWindow.webContents.executeJavaScript(
mw.loadURL('file://' + __dirname + '/app.html'); // eslint-disable-line no-path-concat
mw.webContents.executeJavaScript(
// We use this so that RN can keep relative JSX __source filenames
// but "click to open in editor" still works. js1 passes project roots
// as the argument to DevTools.
'window.devtools.setProjectRoots(' + JSON.stringify(projectRoots) + ')'
);

if (argv.theme) {
mainWindow.webContents.executeJavaScript(
mw.webContents.executeJavaScript(
'window.devtools.setDefaultThemeName(' +
JSON.stringify(defaultThemeName) +
')'
);
}

// Emitted when the window is closed.
mainWindow.on('closed', function() {
mw.on('closed', function() {
mainWindow = null;
});
});
8 changes: 4 additions & 4 deletions shells/browser/shared/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// Running module factories is intentionally delayed until we know the hook exists.
// This is to avoid issues like: https://github.com/facebook/react-devtools/issues/1039

function welcome(event) {
function welcome(event: any) {
if (
event.source !== window ||
event.data.source !== 'relay-devtools-content-script'
Expand All @@ -26,14 +26,14 @@ function welcome(event) {

window.addEventListener('message', welcome);

function setup(hook) {
function setup(hook: any) {
const Agent = require('src/backend/agent').default;
const Bridge = require('src/bridge').default;
const { initBackend } = require('src/backend');

const bridge = new Bridge({
const bridge = new Bridge<any, any>({
listen(fn) {
const listener = event => {
const listener = (event: any) => {
if (
event.source !== window ||
!event.data ||
Expand Down
10 changes: 5 additions & 5 deletions shells/browser/shared/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

/* global chrome */

const ports = {};
const ports: $FlowFixMe = {};

const IS_FIREFOX = navigator.userAgent.indexOf('Firefox') >= 0;

Expand Down Expand Up @@ -50,13 +50,13 @@ function installContentScript(tabId: number) {
);
}

function doublePipe(one, two) {
function doublePipe(one: any, two: any) {
one.onMessage.addListener(lOne);
function lOne(message) {
function lOne(message: any) {
two.postMessage(message);
}
two.onMessage.addListener(lTwo);
function lTwo(message) {
function lTwo(message: any) {
one.postMessage(message);
}
function shutdown() {
Expand All @@ -69,7 +69,7 @@ function doublePipe(one, two) {
two.onDisconnect.addListener(shutdown);
}

function setIconAndPopup(relayBuildType, tabId) {
function setIconAndPopup(relayBuildType: string, tabId: number) {
chrome.browserAction.setIcon({
tabId: tabId,
path: {
Expand Down
6 changes: 3 additions & 3 deletions shells/browser/shared/src/contentScript.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function sayHelloToBackend() {
);
}

function handleMessageFromDevtools(message) {
function handleMessageFromDevtools(message: any) {
window.postMessage(
{
source: 'relay-devtools-content-script',
Expand All @@ -32,7 +32,7 @@ function handleMessageFromDevtools(message) {
);
}

function handleMessageFromPage(evt) {
function handleMessageFromPage(evt: any) {
if (
evt.source === window &&
evt.data &&
Expand Down Expand Up @@ -76,7 +76,7 @@ sayHelloToBackend();
// In the event of a page reload, the content script might be loaded before the backend is injected.
// Because of this we need to poll the backend until it has been initialized.
if (!backendInitialized) {
const intervalID = setInterval(() => {
const intervalID: IntervalID = setInterval(() => {
if (backendInitialized || backendDisconnected) {
clearInterval(intervalID);
} else {
Expand Down
2 changes: 1 addition & 1 deletion shells/browser/shared/src/injectGlobalHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import nullthrows from 'nullthrows';
import { installHook } from 'src/hook';

function injectCode(code) {
function injectCode(code: string) {
const script = document.createElement('script');
script.textContent = code;

Expand Down
8 changes: 4 additions & 4 deletions shells/dev/relay-app/FriendsList/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { graphql, QueryRenderer } from 'react-relay';
import createInBrowserNetwork from './createInBrowserNetwork';
import Friends from './Friends';

function createNewEnvironment(configName) {
function createNewEnvironment(configName: string) {
const source = new RecordSource();
const store = new Store(source);
var environment = new Environment({
Expand All @@ -37,7 +37,7 @@ export type Item = {|

type Props = {||};

export default function App(props: Props) {
export default function App(props: Props): React$MixedElement {
// Add initial environment to environmentList
const [environmentList, updateEnvironmentList] = useState({
'Example Environment': initialEnvironment,
Expand All @@ -53,12 +53,12 @@ export default function App(props: Props) {

updateEnvironmentList({
...environmentList,
[newEnvironment.configName]: newEnvironment,
[(newEnvironment.configName: $FlowFixMe)]: newEnvironment,
});
}, [environmentList]);

const selectNewEnvironment = useCallback(
e => {
(e: any) => {
setCurrentEnvironment(environmentList[e.target.value]);
},
[environmentList]
Expand Down
8 changes: 4 additions & 4 deletions shells/dev/relay-app/FriendsList/FriendCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import React from 'react';
import { graphql, createFragmentContainer } from 'react-relay';
import styles from './FriendCard.css';
import type { FriendCard_user } from './__generated__/FriendCard_user.graphql';
import type { FriendCard_user$data } from './__generated__/FriendCard_user.graphql';

type Props = {|
+user: FriendCard_user,
+user: FriendCard_user$data,
|};

export default createFragmentContainer(
export default (createFragmentContainer(
function FriendCard(props: Props) {
return (
<div className={styles.Card}>
Expand All @@ -39,4 +39,4 @@ export default createFragmentContainer(
}
`,
}
);
): $FlowFixMe);
12 changes: 6 additions & 6 deletions shells/dev/relay-app/FriendsList/Friends.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import React, { Fragment } from 'react';
import { graphql, createPaginationContainer } from 'react-relay';
import FriendCard from './FriendCard';
import styles from './Friends.css';
import type { Friends_user } from './__generated__/Friends_user.graphql';
import type { RelayProps } from 'react-relay';
import type { Friends_user$data } from './__generated__/Friends_user.graphql';
import type { $RelayProps } from 'react-relay';

type Props = {|
+user: Friends_user,
+relay: RelayProps,
+user: Friends_user$data,
+relay: $RelayProps<any>,
|};

export default createPaginationContainer(
export default (createPaginationContainer(
function Friends(props: Props) {
const edges = Array.isArray(props.user.friends?.edges)
? props.user.friends?.edges
Expand Down Expand Up @@ -92,4 +92,4 @@ export default createPaginationContainer(
}
`,
}
);
): $FlowFixMe);
Loading

0 comments on commit b784ca0

Please sign in to comment.