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

tools/test: Fix regression that disabled ESLint #5569

Merged
merged 8 commits into from Nov 30, 2022
14 changes: 7 additions & 7 deletions src/action-sheets/index.js
Expand Up @@ -44,7 +44,7 @@ import { Role, type RoleT } from '../api/permissionsTypes';
import { roleIsAtLeast } from '../permissionSelectors';
import { kNotificationBotEmail } from '../api/constants';
import type { AppNavigationMethods } from '../nav/AppNavigator';
import typeof ComposeBox from '../compose/ComposeBox';
import { type ImperativeHandle as ComposeBoxImperativeHandle } from '../compose/ComposeBox';

// TODO really this belongs in a libdef.
export type ShowActionSheetWithOptions = (
Expand Down Expand Up @@ -94,7 +94,7 @@ type MessageArgs = {
dispatch: Dispatch,
startEditMessage: (editMessage: EditMessage) => void,
setDoNotMarkMessagesAsRead: boolean => void,
composeBoxRefCurrent: React$ElementRef<ComposeBox> | null,
composeBoxImperativeHandle: ComposeBoxImperativeHandle | null,
navigation: AppNavigationMethods,
_: GetText,
...
Expand Down Expand Up @@ -133,12 +133,12 @@ const reply = {
const quoteAndReply = {
title: 'Quote and reply',
errorMessage: 'Quote-and-reply failed',
action: async ({ message, composeBoxRefCurrent }) => {
if (!composeBoxRefCurrent) {
action: async ({ message, composeBoxImperativeHandle }) => {
if (!composeBoxImperativeHandle) {
logging.error("quoteAndReply button pressed when it shouldn't have appeared in the UI");
return;
}
return composeBoxRefCurrent.doQuoteAndReply(message);
return composeBoxImperativeHandle.doQuoteAndReply(message);
},
};

Expand Down Expand Up @@ -807,7 +807,7 @@ export const showMessageActionSheet = (args: {|
callbacks: {|
dispatch: Dispatch,
startEditMessage: (editMessage: EditMessage) => void,
composeBoxRefCurrent: React$ElementRef<ComposeBox> | null,
composeBoxImperativeHandle: ComposeBoxImperativeHandle | null,
navigation: AppNavigationMethods,
_: GetText,
setDoNotMarkMessagesAsRead: boolean => void,
Expand All @@ -833,7 +833,7 @@ export const showMessageActionSheet = (args: {|
backgroundData,
message,
narrow,
canStartQuoteAndReply: callbacks.composeBoxRefCurrent !== null,
canStartQuoteAndReply: callbacks.composeBoxImperativeHandle !== null,
}),
args: { ...backgroundData, ...callbacks, message, narrow },
});
Expand Down
8 changes: 5 additions & 3 deletions src/chat/ChatScreen.js
@@ -1,5 +1,5 @@
/* @flow strict-local */
import React, { useCallback, useContext, useState } from 'react';
import React, { useCallback, useContext } from 'react';
import type { Node } from 'react';
import { useIsFocused } from '@react-navigation/native';

Expand All @@ -15,7 +15,9 @@ import NoMessages from '../message/NoMessages';
import FetchError from './FetchError';
import InvalidNarrow from './InvalidNarrow';
import { fetchMessagesInNarrow } from '../message/fetchActions';
import ComposeBox from '../compose/ComposeBox';
import ComposeBox, {
type ImperativeHandle as ComposeBoxImperativeHandle,
} from '../compose/ComposeBox';
import UnreadNotice from './UnreadNotice';
import { showComposeBoxOnNarrow, caseNarrowDefault, keyFromNarrow } from '../utils/narrow';
import { getLoading, getSession } from '../directSelectors';
Expand Down Expand Up @@ -144,7 +146,7 @@ export default function ChatScreen(props: Props): Node {
const sayNoMessages = messages.length === 0 && !isFetching;

const showComposeBox = showComposeBoxOnNarrow(narrow) && !showMessagePlaceholders;
const composeBoxRef = React.useRef<React$ElementRef<typeof ComposeBox> | null>(null);
const composeBoxRef = React.useRef<ComposeBoxImperativeHandle | null>(null);

const auth = useSelector(getAuth);
const dispatch = useDispatch();
Expand Down
2 changes: 1 addition & 1 deletion src/compose/ComposeBox.js
Expand Up @@ -111,7 +111,7 @@ export type ValidationError =
/**
* Functions expected to be called using a ref to this component.
*/
type ImperativeHandle = {|
export type ImperativeHandle = {|
/**
* Take a message ID, fetch its raw Markdown content, and put it in the
* compose box with proper formatting.
Expand Down
6 changes: 3 additions & 3 deletions src/webview/MessageList.js
Expand Up @@ -41,7 +41,7 @@ import { type BackgroundData, getBackgroundData } from './backgroundData';
import { ensureUnreachable } from '../generics';
import SinglePageWebView from './SinglePageWebView';
import { usePrevious } from '../reactUtils';
import typeof ComposeBox from '../compose/ComposeBox';
import { type ImperativeHandle as ComposeBoxImperativeHandle } from '../compose/ComposeBox';

/**
* The actual React props for the MessageList component.
Expand All @@ -54,7 +54,7 @@ type OuterProps = $ReadOnly<{|
startEditMessage: (editMessage: EditMessage) => void,

// Careful: We expect this prop to be mutable, which is unusual.
composeBoxRef: {| current: React$ElementRef<ComposeBox> | null |},
composeBoxRef: {| current: ComposeBoxImperativeHandle | null |},
|}>;

/**
Expand Down Expand Up @@ -320,7 +320,7 @@ export default function MessageList(outerProps: OuterProps): React.Node {
handleWebViewOutboundEvent(propsRef.current, navigation, eventData);
}
},
[sendInboundEvents],
[sendInboundEvents, navigation],
);

// We compute the page contents as an HTML string just once (*), on this
Expand Down
2 changes: 1 addition & 1 deletion src/webview/handleOutboundEvents.js
Expand Up @@ -258,7 +258,7 @@ const handleLongPress = (args: {|
// that an action-sheet button press will act on values that were
// current when the action sheet was opened:
// https://github.com/zulip/zulip-mobile/pull/5554#discussion_r1027004559
composeBoxRefCurrent: composeBoxRef.current,
composeBoxImperativeHandle: composeBoxRef.current,

navigation,
_,
Expand Down
58 changes: 29 additions & 29 deletions tools/test
Expand Up @@ -68,43 +68,43 @@ EOF
exit 2
}

coverage=
files=branch
platform=sloppy
fix=
suites=()
opt_coverage=
opt_files=branch
opt_platform=sloppy
opt_fix=
opt_suites=()
while (( $# )); do
case "$1" in
--coverage) coverage=1; shift;;
--diff) shift; files=diff:"$1"; shift;;
--all-files) files=all; shift;;
--coverage) opt_coverage=1; shift;;
--diff) shift; opt_files=diff:"$1"; shift;;
--all-files) opt_files=all; shift;;
--platform)
shift;
case "$1" in
ios|android|both|sloppy) platform="$1";;
ios|android|both|sloppy) opt_platform="$1";;
*) usage;;
esac
shift
;;
--all) files=all; platform=both; shift;;
--fix) fix=1; shift;;
--all) opt_files=all; opt_platform=both; shift;;
--fix) opt_fix=1; shift;;
native|flow|lint|jest|prettier|deps|tsflower)
suites+=("$1"); shift;;
opt_suites+=("$1"); shift;;
*) usage;;
esac
done

if [ -z "$suites" ]; then
if [ -z "$opt_suites" ]; then
# This default doesn't have to be the complete list; just be sure to
# document in the usage message any suites that it skips.
suites=(native flow lint jest prettier deps tsflower)
opt_suites=(native flow lint jest prettier deps tsflower)
fi

files_base_commit=
case "$files" in
case "$opt_files" in
all) ;;
branch) files_base_commit="$(tools/git base)";;
diff:*) files_base_commit="${files#diff:}";;
diff:*) files_base_commit="${opt_files#diff:}";;
esac


Expand All @@ -115,12 +115,12 @@ cd "$rootdir"

PATH=node_modules/.bin:"$PATH"

# Intersect $files with the set of our JS files in src/.
# Intersect $opt_files with the set of our JS files in src/.
#
# Prints a list of newline-terminated paths; either files, or
# directories meaning their whole subtrees.
files_js() {
case "$files" in
case "$opt_files" in
all)
echo src/
;;
Expand All @@ -132,7 +132,7 @@ files_js() {

# True just if $files intersects the given set of paths.
files_check() {
case "$files" in
case "$opt_files" in
all)
;;
branch | diff:*)
Expand Down Expand Up @@ -166,12 +166,12 @@ run_native_ios() {
}

run_native() {
if [[ $platform == android || $platform == both || $platform == sloppy ]]; then
if [[ $opt_platform == android || $opt_platform == both || $opt_platform == sloppy ]]; then
echo 'Running Android native tests...';
run_native_android || return
fi

if [[ $platform == ios || $platform == both || $platform == sloppy ]]; then
if [[ $opt_platform == ios || $opt_platform == both || $opt_platform == sloppy ]]; then
# TODO: Run if on macOS; otherwise, echo that these tests are
# skipped because they can't be run.

Expand All @@ -185,15 +185,15 @@ run_lint() {
files=( $(files_js) )
files=( $(apply_eslintignore "${files[@]}") )
(( ${#files[@]} )) || return 0
eslint ${fix:+--fix} --report-unused-disable-directives --max-warnings=0 "${files[@]}"
eslint ${opt_fix:+--fix} --report-unused-disable-directives --max-warnings=0 "${files[@]}"
}

run_jest() {
# Unlike some others, this inspects "$files" for itself.
# Unlike some others, this inspects "$opt_files" for itself.
local jest_args=()
case "$files" in
case "$opt_files" in
all)
if [ -n "$coverage" ]; then
if [ -n "$opt_coverage" ]; then
jest_args+=( --coverage )
fi
;;
Expand All @@ -209,7 +209,7 @@ run_jest() {
esac

local platforms=( ios android )
case "$platform" in
case "$opt_platform" in
ios) jest_args+=( --selectProjects ios );;
android) jest_args+=( --selectProjects android );;
both) jest_args+=( --selectProjects ios android );;
Expand All @@ -225,7 +225,7 @@ run_jest() {

run_prettier() {
local patterns
case "$files" in
case "$opt_files" in
all)
# The prettier-eslint CLI won't take directories;
# but it's happy to take glob patterns, and supports `**`.
Expand All @@ -242,7 +242,7 @@ run_prettier() {
# Workaround for https://github.com/prettier/prettier-eslint-cli/issues/205
patterns=( "${patterns[@]/#/$rootdir/}" )
prettier-eslint \
${fix:+--write} \
${opt_fix:+--write} \
--list-different \
--eslint-config-path "${rootdir}"/tools/formatting.eslintrc.yaml \
"${patterns[@]}"
Expand Down Expand Up @@ -285,7 +285,7 @@ run_tsflower() {
}

failed=()
for suite in "${suites[@]}"; do
for suite in "${opt_suites[@]}"; do
echo "Running $suite..."
case "$suite" in
native) run_native ;;
Expand Down