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

Improve frontend lint settings #969

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions frontend/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"no-restricted-imports": ["error", {
"patterns": ["*/**", "!~/**", "!./**", "!@*/**", "!react*/**"]
}],
"no-console": "error",
"@typescript-eslint/explicit-function-return-type": "off",
"@typescript-eslint/interface-name-prefix": "off",
"@typescript-eslint/no-var-requires": "off",
Expand Down
1 change: 1 addition & 0 deletions frontend/src/app/HeaderTools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const HeaderTools: React.FC<HeaderToolsProps> = ({ onNotificationsClick }) => {
const handleLogout = () => {
setUserMenuOpen(false);
logout().then(() => {
/* eslint-disable-next-line no-console */
console.log('logged out');
window.location.reload();
});
Expand Down
1 change: 1 addition & 0 deletions frontend/src/app/appUtils.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export const logout = (): Promise<unknown> =>
/* eslint-disable-next-line no-console */
fetch('/oauth/sign_out').catch((err) => console.error('Error logging out', err));
1 change: 1 addition & 0 deletions frontend/src/app/useApplicationSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const useApplicationSettings = (): {
if (e?.message?.includes('Error getting Oauth Info for user')) {
// NOTE: this endpoint only requests oauth because of the security layer, this is not an ironclad use-case
// Something went wrong on the server with the Oauth, let us just log them out and refresh for them
/* eslint-disable-next-line no-console */
console.error(
'Something went wrong with the oauth token, please log out...',
e.message,
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/app/useTimeBasedRefresh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const useTimeBasedRefresh = (): SetTime => {
const lastDate = new Date(ref.current.lastRefreshTimestamp);
const setNewDateString = ref.current.setLastRefreshTimestamp;

/* eslint-disable no-console */
// Print into the console in case we are not refreshing or the browser has preserve log enabled
console.warn('Attempting to re-trigger an auto refresh');
console.log('Last refresh was on:', lastDate);
Expand All @@ -39,6 +40,7 @@ const useTimeBasedRefresh = (): SetTime => {
`We should have refreshed but it appears the last time we auto-refreshed was less than an hour ago. '${KEY_NAME}' session storage setting can be cleared for this to refresh again within the hour from the last refresh.`,
);
}
/* eslint-enable no-console */
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm... wonder if we can disable block disable/enables -- seems easy to abuse.

}, []);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const useBrowserStorage = <T,>(
setStringValue(storageKey, value, isSessionStorage);
return true;
}
/* eslint-disable-next-line no-console */
console.error('Was not a string value provided, cannot stringify');
return false;
},
Expand Down Expand Up @@ -96,6 +97,7 @@ export const BrowserStorageContextProvider: React.FC<BrowserStorageContextProvid
try {
return JSON.parse(value);
} catch (e) {
/* eslint-disable-next-line no-console */
console.warn(`Failed to parse storage value "${key}"`);
return null;
}
Expand All @@ -115,6 +117,7 @@ export const BrowserStorageContextProvider: React.FC<BrowserStorageContextProvid

return true;
} catch (e) {
/* eslint-disable-next-line no-console */
console.warn(
'Could not store a value because it was requested to be stringified but was an invalid value for stringification.',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const NotebookLogoutRedirect: React.FC = () => {
if (cancelled) {
return;
}
/* eslint-disable-next-line no-console */
console.error(e);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ const SpawnerPage: React.FC = () => {
setVariableRows([...fetchedVariableRowsConfigMap, ...fetchedVariableRowsSecret]);
}
};
/* eslint-disable-next-line no-console */
mapEnvironmentVariableRows().catch((e) => console.error(e));
return () => {
cancelled = true;
Expand Down Expand Up @@ -335,6 +336,7 @@ const SpawnerPage: React.FC = () => {
handleNotebookAction().catch((e) => {
setCreateInProgress(false);
hideStartShown();
/* eslint-disable-next-line no-console */
console.error('Error submitting resources around starting a notebook', e);
setSubmitError(e);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ const useGPUSetting = (
setFetching(false);
setAreGpusAvailable(false);
setGpuSize(0);
console.error(e);
notification.error('Failed to fetch GPU', e.message);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const useSpawnerNotebookModalState = (
if (!createInProgress) {
// We are not creating, make sure the Notebook is stopped
stopNotebook().catch(() => {
/* eslint-disable-next-line no-console */
console.error('Failed to stop notebook on refresh');
});
} else {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/projects/notebook/ListNotebookState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const ListNotebookState: React.FC<ListNotebookStateProps> = ({
case 'status':
return <NotebookStatusToggle notebookState={state} doListen />;
default:
/* eslint-disable-next-line no-console */
console.error('Unknown show type', show);
return <>-</>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const useRefreshNotebookUntilStart = (
const { isRunning, refresh } = lastNotebookState.current;
if (!isRunning) {
refresh().catch((e) => {
/* eslint-disable-next-line no-console */
console.error('Error refreshing, stopping notebook refresh', e);
setWatchingForNotebook(false);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const useWatchNotebookEvents = (
setNoteBookEvents(data);
})
.catch((e) => {
/* eslint-disable-next-line no-console */
console.error('Error fetching notebook events', e);
clear();
});
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/pages/projects/screens/projects/ProjectTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const ProjectTable: React.FC<ProjectTableProps> = ({

refreshProjects()
.then(() => setRefreshIds((ids) => ids.filter((id) => id !== refreshId)))
/* eslint-disable-next-line no-console */
.catch((e) => console.error('Failed refresh', e));
}}
editProjectData={editData}
Expand All @@ -79,6 +80,7 @@ const ProjectTable: React.FC<ProjectTableProps> = ({
onClose={(deleted) => {
setDeleteData(undefined);
if (deleted) {
/* eslint-disable-next-line no-console */
refreshProjects().catch((e) => console.error('Failed refresh', e));
}
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export const useNotebookEnvVariables = (
if (notebook) {
fetchNotebookEnvVariables(notebook)
.then((envVars) => setEnvVariables(envVars))
/* eslint-disable-next-line no-console */
.catch((e) => console.error('Reading environment variables failed: ', e));
}
}, [notebook]);
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/projects/screens/spawner/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export const createConfigMapsAndSecretsForNotebook = async (
return Promise.all(creatingPromises)
.then((results: (ConfigMapKind | SecretKind)[]) => getEnvFromList(results, []))
.catch((e) => {
/* eslint-disable-next-line no-console */
console.error('Creating environment variables failed: ', e);
throw e;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export const getImageVersionDependencies = (
} catch (e) {
if (depString.includes('[')) {
// It was intended to be an array but failed to parse, log the error
/* eslint-disable-next-line no-console */
console.error(`JSON parse error when parsing ${imageVersion.name}`);
}
dependencies = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const useNotebookRootStorage = (notebook?: NotebookKind): PersistentVolumeClaimK
(volumeMount) => volumeMount.mountPath === ROOT_MOUNT_PATH,
);
if (!volumeMount) {
/* eslint-disable-next-line no-console */
console.error('No storage mounted on root path');
setPvc(undefined);
} else {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/services/notebookService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const getNotebookAndStatus = (
if (e.response.status === 404) {
return { notebook, isRunning: false };
}

/* eslint-disable-next-line no-console */
console.error(
'Checking notebook status failed, falling back on notebook check logic',
e.response.data.message,
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/utilities/notebookControllerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export const getNotebookControllerUserState = (
if (usernameTranslate(loggedInUser) === notebookLabelUser) {
user = loggedInUser;
} else {
/* eslint-disable-next-line no-console */
console.error('Could not get full user data');
return null;
}
Expand Down Expand Up @@ -203,6 +204,7 @@ export const useNotebookRedirectLink = (): (() => Promise<string>) => {
if (!routeName) {
// At time of call, if we do not have a route name, we are too late
// This should *never* happen, somehow the modal got here before the Notebook had a name!?
/* eslint-disable-next-line no-console */
console.error('Unable to determine why there was no route -- notebook did not have a name');
return Promise.reject();
}
Expand All @@ -218,6 +220,7 @@ export const useNotebookRedirectLink = (): (() => Promise<string>) => {
resolve(backupRoute);
return;
}
/* eslint-disable-next-line no-console */
console.warn('Unable to get the route. Re-polling.', e);
if (fetchCountRef.current <= 0) {
fetchCountRef.current--;
Expand All @@ -229,6 +232,7 @@ export const useNotebookRedirectLink = (): (() => Promise<string>) => {
};

call(resolve, () => {
/* eslint-disable-next-line no-console */
console.error(
'Could not fetch route over several tries, See previous warnings for a history of why each failed call.',
);
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/utilities/segmentIOUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const fireTrackingEvent = (
): void => {
const clusterID = window.clusterID ?? '';
if (DEV_MODE) {
/* eslint-disable-next-line no-console */
console.log(
`Telemetry event triggered: ${eventType}${
properties ? ` - ${JSON.stringify(properties)}` : ''
Expand Down Expand Up @@ -36,6 +37,7 @@ export const initSegment = async (props) => {
return;
}
if (analytics.invoked) {
/* eslint-disable-next-line no-console */
window.console && console.error && console.error('Segment snippet included twice.');
} else {
analytics.invoked = true;
Expand Down