Skip to content

Commit

Permalink
[ui] Lint against window.open
Browse files Browse the repository at this point in the history
  • Loading branch information
hellendag committed Nov 13, 2024
1 parent 9713613 commit 0ee85cb
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 14 deletions.
15 changes: 15 additions & 0 deletions js_modules/dagster-ui/packages/eslint-config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,21 @@ module.exports = {
},
],
'no-alert': 'error',
'no-restricted-properties': [
'error',
{
object: 'window',
property: 'open',
message: 'Please use the `useOpenInNewTab` hook instead.',
},
],
'no-restricted-globals': [
'error',
{
name: 'open',
message: 'Please use the `useOpenInNewTab` hook instead.',
},
],
'no-unused-vars': 'off', // or "@typescript-eslint/no-unused-vars": "off",
'unused-imports/no-unused-imports': 'error',
'unused-imports/no-unused-vars': [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {AssetGraphViewType} from '../asset-graph/Utils';
import {AssetLocation} from '../asset-graph/useFindAssetLocation';
import {AssetGroupSelector} from '../graphql/types';
import {useDocumentTitle} from '../hooks/useDocumentTitle';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';
import {RepositoryLink} from '../nav/RepositoryLink';
import {
ExplorerPath,
Expand Down Expand Up @@ -48,6 +49,7 @@ export const AssetGroupRoot = ({
const history = useHistory();

useDocumentTitle(`Asset Group: ${groupName}`);
const openInNewTab = useOpenInNewTab();

const groupPath = workspacePathFromAddress(repoAddress, `/asset-groups/${groupName}`);
const groupSelector = useMemo(
Expand Down Expand Up @@ -83,12 +85,12 @@ export const AssetGroupRoot = ({
path = assetDetailsPathForKey(node.assetKey, {view: 'definition'});
}
if (e.metaKey) {
window.open(path, '_blank');
openInNewTab(path);
} else {
history.push(path);
}
},
[history],
[history, openInNewTab],
);

const fetchOptions = React.useMemo(() => ({groupSelector}), [groupSelector]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {DEFAULT_MAX_ZOOM, SVGViewport} from '../graph/SVGViewport';
import {useAssetLayout} from '../graph/asyncGraphLayout';
import {isNodeOffscreen} from '../graph/common';
import {AssetKeyInput} from '../graphql/types';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';

export type AssetNodeLineageGraphProps = {
assetKey: AssetKeyInput;
Expand All @@ -30,6 +31,7 @@ export const AssetNodeLineageGraph = ({
assetGraphData,
params,
}: AssetNodeLineageGraphProps) => {
const openInNewTab = useOpenInNewTab();
const assetGraphId = toGraphId(assetKey);

const {allGroups, groupedAssets} = useMemo(() => {
Expand Down Expand Up @@ -60,7 +62,7 @@ export const AssetNodeLineageGraph = ({
lineageDepth: 1,
});
if (e.metaKey) {
window.open(document.location.host + path);
openInNewTab(path);
} else {
history.push(path);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {AssetGraphViewType} from '../asset-graph/Utils';
import {AssetGraphFetchScope} from '../asset-graph/useAssetGraphData';
import {AssetLocation} from '../asset-graph/useFindAssetLocation';
import {useDocumentTitle} from '../hooks/useDocumentTitle';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';
import {ExplorerPath} from '../pipelines/PipelinePathUtils';

interface AssetGroupRootParams {
Expand All @@ -26,6 +27,7 @@ export const AssetsGlobalGraphRoot = () => {
const history = useHistory();

useDocumentTitle(`Global Asset Lineage`);
const openInNewTab = useOpenInNewTab();

const onChangeExplorerPath = useCallback(
(path: ExplorerPath, mode: 'push' | 'replace') => {
Expand All @@ -41,12 +43,12 @@ export const AssetsGlobalGraphRoot = () => {
(e: Pick<React.MouseEvent<any>, 'metaKey'>, node: AssetLocation) => {
const path = assetDetailsPathForKey(node.assetKey, {view: 'definition'});
if (e.metaKey) {
window.open(path, '_blank');
openInNewTab(path);
} else {
history.push(path);
}
},
[history],
[history, openInNewTab],
);

const fetchOptions = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import {useContext} from 'react';
import {useCallback, useContext} from 'react';

import {AppContext} from '../app/AppContext';

// Open a path in a new tab. Incorporates the base path to ensure that
// the link opens to the appropriate deployment.
export const useOpenInNewTab = () => {
const {basePath} = useContext(AppContext);
return (path: string) => {
window.open(`${basePath}${path}`, '_blank');
};
return useCallback(
(path: string) => {
// eslint-disable-next-line no-restricted-properties
window.open(`${basePath}${path}`, '_blank');
},
[basePath],
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {RenderTooltipFn, renderInsightsChartTooltip} from './renderInsightsChart
import {BarDatapoint, BarValue, DatapointType, ReportingUnitType} from './types';
import {TimeContext} from '../app/time/TimeContext';
import {useRGBColorsForTheme} from '../app/useRGBColorsForTheme';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';
import {useFormatDateTime} from '../ui/useFormatDateTime';

ChartJS.register(CategoryScale, LinearScale, BarElement, Tooltip, Filler);
Expand Down Expand Up @@ -55,6 +56,7 @@ export const InsightsBarChart = (props: Props) => {

const rgbColors = useRGBColorsForTheme();
const [canShowSpinner, setCanShowSpinner] = React.useState(false);
const openInNewTab = useOpenInNewTab();

React.useEffect(() => {
const timer = setTimeout(() => {
Expand Down Expand Up @@ -103,12 +105,12 @@ export const InsightsBarChart = (props: Props) => {
if (element) {
const whichBar = values[element.index];
if (whichBar?.href) {
window.open(whichBar.href);
openInNewTab(whichBar.href);
}
return;
}
},
[values],
[values, openInNewTab],
);

const renderTooltipFn: RenderTooltipFn = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {AssetGraphViewType} from '../asset-graph/Utils';
import {AssetLocation} from '../asset-graph/useFindAssetLocation';
import {assetDetailsPathForKey} from '../assets/assetDetailsPathForKey';
import {useDocumentTitle} from '../hooks/useDocumentTitle';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';
import {METADATA_ENTRY_FRAGMENT} from '../metadata/MetadataEntryFragment';
import {Loading} from '../ui/Loading';
import {buildPipelineSelector} from '../workspace/WorkspaceContext/util';
Expand All @@ -36,6 +37,7 @@ export const PipelineExplorerSnapshotRoot = () => {
const {pipelineName, snapshotId} = explorerPath;
const history = useHistory();

const openInNewTab = useOpenInNewTab();
useDocumentTitle(`Snapshot: ${pipelineName}${snapshotId ? `@${snapshotId.slice(0, 8)}` : ''}`);

return (
Expand All @@ -47,7 +49,7 @@ export const PipelineExplorerSnapshotRoot = () => {
onNavigateToSourceAssetNode={(e, {assetKey}) => {
const path = assetDetailsPathForKey(assetKey);
if (e.metaKey) {
window.open(path, '_blank');
openInNewTab(path);
} else {
history.push(assetDetailsPathForKey(assetKey));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {useTrackPageView} from '../app/analytics';
import {tokenForAssetKey} from '../asset-graph/Utils';
import {AssetLocation} from '../asset-graph/useFindAssetLocation';
import {assetDetailsPathForKey} from '../assets/assetDetailsPathForKey';
import {useOpenInNewTab} from '../hooks/useOpenInNewTab';
import {isThisThingAJob, useRepository} from '../workspace/WorkspaceContext/util';
import {RepoAddress} from '../workspace/types';
import {workspacePathFromAddress} from '../workspace/workspacePath';
Expand All @@ -31,6 +32,7 @@ export const PipelineOverviewRoot = (props: Props) => {
const pathStr = (params as any)['0'];
const explorerPath = useMemo(() => explorerPathFromString(pathStr), [pathStr]);

const openInNewTab = useOpenInNewTab();
const repo = useRepository(repoAddress);
const isJob = isThisThingAJob(repo, explorerPath.pipelineName);

Expand Down Expand Up @@ -58,7 +60,7 @@ export const PipelineOverviewRoot = (props: Props) => {
// but there can be a description.
const path = assetDetailsPathForKey(node.assetKey, {view: 'definition'});
if (e.metaKey) {
window.open(path, '_blank');
openInNewTab(path);
} else {
history.push(path);
}
Expand All @@ -80,7 +82,7 @@ export const PipelineOverviewRoot = (props: Props) => {
),
});
},
[explorerPath, history, location.search],
[explorerPath, history, location.search, openInNewTab],
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export const RunHeaderActions = ({run, isJob}: {run: RunFragment; isJob: boolean
<MenuItem
text="Download debug file"
icon="download_for_offline"
// eslint-disable-next-line no-restricted-properties
onClick={() => window.open(`${rootServerURI}/download_debug/${run.id}`)}
/>
</Tooltip>
Expand Down

0 comments on commit 0ee85cb

Please sign in to comment.