Skip to content

Commit ba6b6b3

Browse files
committed
url [nfc]: Factor out helper tryParseUrl.
This allows callers like `canUseNativeAppleFlow` to use plain conditionals instead of try/catch, and use more `const` so that the type-checker is better able to see when a variable can no longer be a value like `undefined`. In this case, for example, if instead of this change a `return false` is simply added to the `catch` block, then Flow still believes the `host !== undefined` check below is necessary (and that without it the `host.endsWith(…)` call is ill-typed), because `host` is a mutable binding and Flow doesn't see how to prove that it doesn't get set back to `undefined`. With `const` everywhere, the logic to rule out such possibilities is more straightforward and the type-checker is able to see it.
1 parent 9c13381 commit ba6b6b3

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

src/start/AuthScreen.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import styles from '../styles';
2828
import { Centerer, Screen, ZulipButton } from '../common';
2929
import { getCurrentRealm } from '../selectors';
3030
import RealmInfo from './RealmInfo';
31-
import { getFullUrl, encodeParamsForUrl } from '../utils/url';
31+
import { getFullUrl, encodeParamsForUrl, tryParseUrl } from '../utils/url';
3232
import * as webAuth from './webAuth';
3333
import { loginSuccess, navigateToDev, navigateToPassword } from '../actions';
3434
import IosCompliantAppleAuthButton from './IosCompliantAppleAuthButton';
@@ -276,12 +276,11 @@ class AuthScreen extends PureComponent<Props> {
276276
return false;
277277
}
278278

279-
let host: string | void;
280-
try {
281-
host = new WhatwgURL(this.props.realm).host;
282-
} catch (e) {
279+
const host = tryParseUrl(this.props.realm)?.host;
280+
if (host === undefined) {
283281
// `this.props.realm` invalid.
284282
// TODO: Check this much sooner.
283+
return false;
285284
}
286285

287286
// The native flow for Apple auth assumes that the app and the server
@@ -291,9 +290,7 @@ class AuthScreen extends PureComponent<Props> {
291290
//
292291
// (For other realms, we'll simply fall back to the web flow, which
293292
// handles things appropriately without relying on that assumption.)
294-
return config.appOwnDomains.some(
295-
domain => host !== undefined && (host === domain || host.endsWith(`.${domain}`)),
296-
);
293+
return config.appOwnDomains.some(domain => host === domain || host.endsWith(`.${domain}`));
297294
};
298295

299296
handleAuth = async (method: AuthenticationMethodDetails) => {

src/utils/url.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/* @flow strict-local */
22
import urlRegex from 'url-regex';
3+
import { URL as WhatwgURL } from 'react-native-url-polyfill';
34

45
import type { Auth } from '../types';
56
import { getAuthHeaders } from '../api/transport';
@@ -31,6 +32,15 @@ export const encodeParamsForUrl = (params: UrlParams): string =>
3132
.map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value.toString())}`)
3233
.join('&');
3334

35+
/** Just like `new WhatwgURL`, but on error return undefined instead of throwing. */
36+
export const tryParseUrl = (url: string, base?: string | WhatwgURL): WhatwgURL | void => {
37+
try {
38+
return new WhatwgURL(url, base);
39+
} catch (e) {
40+
return undefined;
41+
}
42+
};
43+
3444
/**
3545
* Turn a relative or absolute URL into an absolute URL.
3646
*

0 commit comments

Comments
 (0)