Add lint rule to disallow importing Route from react-router-dom (#24176)

## Summary & Motivation

Adds a lint rule to disallow importing Route directly from
react-router-dom. Instead it should be imported from our custom Route
component that wraps the one from react-router-dom

## How I Tested These Changes

jest test + ran `yarn lint --fix` which created the updated imports in
this PR

## Changelog [New | Bug | Docs]
NOCHANGELOG

js_modules/dagster-ui/packages/eslint-config/.eslintrc.js

@@ -1,3 +1,8 @@
 module.exports = {
   extends: './index.js',
+  overrides: {
+    rules: {
+      '@typescript-eslint/no-var-requires': 'off',
+    },
+  },
 };

js_modules/dagster-ui/packages/eslint-config/rules/__tests__/missing-graphql-variables-type.test.js

@@ -10,7 +10,7 @@ jest.mock('fs');
 const fs = require('fs');
 
 // @ts-expect-error - using require because this package isn't setup for import declarations
-const {rule} = require('../missing-graphql-variables-type');
+const rule = require('../missing-graphql-variables-type');
 
 fs.readFileSync = (path) => {
   const api = path.includes('Query')

js_modules/dagster-ui/packages/eslint-config/rules/__tests__/no-apollo-client.js

@@ -0,0 +1,32 @@
+/* eslint-disable */
+const {ESLintUtils, AST_NODE_TYPES} = require('@typescript-eslint/utils');
+
+const ruleTester = new ESLintUtils.RuleTester({
+  parser: '@typescript-eslint/parser',
+});
+
+const rule = require('../no-apollo-client');
+
+ruleTester.run('rule', rule, {
+  valid: [
+    `
+      import {useQuery} from '../../apollo-client';
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        import {useQuery} from '@apollo/client';
+      `,
+      output: `
+        import {useQuery} from '@dagster-io/ui-core/apollo-client';
+      `,
+      errors: [
+        {
+          type: AST_NODE_TYPES.ImportDeclaration,
+          messageId: 'useWrappedApolloClient',
+        },
+      ],
+    },
+  ],
+});

js_modules/dagster-ui/packages/eslint-config/rules/__tests__/no-react-router-route.test.js

@@ -0,0 +1,35 @@
+/* eslint-disable */
+const {ESLintUtils, AST_NODE_TYPES} = require('@typescript-eslint/utils');
+
+const ruleTester = new ESLintUtils.RuleTester({
+  parser: '@typescript-eslint/parser',
+});
+
+const rule = require('../no-react-router-route');
+
+ruleTester.run('rule', rule, {
+  valid: [
+    `
+      import {Redirect, Switch} from 'react-router-dom';
+
+      import {Route} from './Route';
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+import {Redirect, Route, Switch} from 'react-router-dom';
+      `,
+      output: `
+import { Route } from '@dagster-io/ui-core/app/Route';
+import {Redirect, Switch} from 'react-router-dom';
+      `,
+      errors: [
+        {
+          type: AST_NODE_TYPES.ImportDeclaration,
+          messageId: 'useDagsterRoute',
+        },
+      ],
+    },
+  ],
+});

js_modules/dagster-ui/packages/eslint-config/rules/index.js

@@ -2,9 +2,10 @@
 const projectName = 'dagster-rules';
 
 const rules = {
-  'missing-graphql-variables-type': require('./missing-graphql-variables-type').rule,
+  'missing-graphql-variables-type': require('./missing-graphql-variables-type'),
   'no-oss-imports': require('./no-oss-imports'),
   'no-apollo-client': require('./no-apollo-client'),
+  'no-react-router-route': require('./no-react-router-route'),
 };
 
 module.exports = {
@@ -16,6 +17,7 @@ module.exports = {
         [`${projectName}/missing-graphql-variables-type`]: 'error',
         [`${projectName}/no-oss-imports`]: 'error',
         [`${projectName}/no-apollo-client`]: 'error',
+        [`${projectName}/no-react-router-route`]: 'error',
       },
     },
   },

js_modules/dagster-ui/packages/eslint-config/rules/missing-graphql-variables-type.js

@@ -25,104 +25,102 @@ const APIToEnding = {
   useLazyQuery: 'LazyQuery',
 };
 
-module.exports = {
-  rule: createRule({
-    create(context) {
-      return {
-        [AST_NODE_TYPES.CallExpression](node) {
-          const callee = node.callee;
-          if (callee.type !== 'Identifier') {
-            return;
-          }
-          // if it's not a useQuery call then ignore
-          if (!APIS.has(callee.name)) {
-            return;
-          }
-          const API = callee.name;
-          const queryType =
-            node.typeParameters && node.typeParameters.params && node.typeParameters.params[0];
-          if (!queryType || queryType.type !== 'TSTypeReference') {
-            return;
-          }
-          if (queryType.typeName.type !== 'Identifier') {
-            return;
-          }
-          const queryName = queryType.typeName.name;
-          // if the type doesn't end with Query then ignore
-          if (!queryName.endsWith(APIToEnding[API])) {
-            return;
-          }
-          const variablesName = queryName + 'Variables';
-          let queryImportSpecifier = null;
-          const importDeclaration = context.getSourceCode().ast.body.find(
-            (node) =>
-              node.type === 'ImportDeclaration' &&
-              node.specifiers.find((node) => {
-                if (node.type === 'ImportSpecifier' && node.local.name === queryName) {
-                  queryImportSpecifier = node;
-                  return true;
-                }
-              }),
-          );
-          const importPath = importDeclaration.source.value;
-          const currentPath = context.getFilename().split('/').slice(0, -1).join('/');
-          const fullPath = path.join(currentPath, importPath + '.ts');
+module.exports = createRule({
+  create(context) {
+    return {
+      [AST_NODE_TYPES.CallExpression](node) {
+        const callee = node.callee;
+        if (callee.type !== 'Identifier') {
+          return;
+        }
+        // if it's not a useQuery call then ignore
+        if (!APIS.has(callee.name)) {
+          return;
+        }
+        const API = callee.name;
+        const queryType =
+          node.typeParameters && node.typeParameters.params && node.typeParameters.params[0];
+        if (!queryType || queryType.type !== 'TSTypeReference') {
+          return;
+        }
+        if (queryType.typeName.type !== 'Identifier') {
+          return;
+        }
+        const queryName = queryType.typeName.name;
+        // if the type doesn't end with Query then ignore
+        if (!queryName.endsWith(APIToEnding[API])) {
+          return;
+        }
+        const variablesName = queryName + 'Variables';
+        let queryImportSpecifier = null;
+        const importDeclaration = context.getSourceCode().ast.body.find(
+          (node) =>
+            node.type === 'ImportDeclaration' &&
+            node.specifiers.find((node) => {
+              if (node.type === 'ImportSpecifier' && node.local.name === queryName) {
+                queryImportSpecifier = node;
+                return true;
+              }
+            }),
+        );
+        const importPath = importDeclaration.source.value;
+        const currentPath = context.getFilename().split('/').slice(0, -1).join('/');
+        const fullPath = path.join(currentPath, importPath + '.ts');
 
-          const graphqlTypeFile = fs.readFileSync(fullPath, {encoding: 'utf8'});
+        const graphqlTypeFile = fs.readFileSync(fullPath, {encoding: 'utf8'});
 
-          // This part is kind of hacky. I should use the parser service to find the identifier
-          // but this is faster then tokenizing the whole file
-          if (
-            !graphqlTypeFile.includes('export type ' + variablesName) &&
-            !graphqlTypeFile.includes('export interface ' + variablesName)
-          ) {
-            return;
-          }
-          // This is a Query type with a generated QueryVariables type. Make sure we're using it
-          const secondType = node.typeParameters.params[1];
-          if (
-            !secondType ||
-            (secondType.type === 'TSTypeReference' &&
-              secondType.typeName.type === 'Identifier' &&
-              secondType.typeName.name !== variablesName)
-          ) {
-            context.report({
-              messageId: 'missing-graphql-variables-type',
-              node,
-              data: {
-                queryType: queryName,
-                variablesType: variablesName,
-                api: API,
-              },
-              *fix(fixer) {
-                if (
-                  !importDeclaration.specifiers.find(
-                    (node) => node.type === 'ImportSpecifier' && node.local.name === variablesName,
-                  )
-                ) {
-                  yield fixer.insertTextAfter(queryImportSpecifier, `, ${variablesName}`);
-                }
-                yield fixer.insertTextAfter(queryType, `, ${variablesName}`);
-              },
-            });
-          }
-        },
-      };
-    },
-    name: 'missing-graphql-variables-type',
-    meta: {
-      fixable: true,
-      docs: {
-        description: 'useQuery is missing QueryVariables parameter.',
-        recommended: 'error',
-      },
-      messages: {
-        'missing-graphql-variables-type':
-          '`{{api}}<{{queryType}}>(...)` should be `{{api}}<{{queryType}},{{variablesType}}>(...)`.',
+        // This part is kind of hacky. I should use the parser service to find the identifier
+        // but this is faster then tokenizing the whole file
+        if (
+          !graphqlTypeFile.includes('export type ' + variablesName) &&
+          !graphqlTypeFile.includes('export interface ' + variablesName)
+        ) {
+          return;
+        }
+        // This is a Query type with a generated QueryVariables type. Make sure we're using it
+        const secondType = node.typeParameters.params[1];
+        if (
+          !secondType ||
+          (secondType.type === 'TSTypeReference' &&
+            secondType.typeName.type === 'Identifier' &&
+            secondType.typeName.name !== variablesName)
+        ) {
+          context.report({
+            messageId: 'missing-graphql-variables-type',
+            node,
+            data: {
+              queryType: queryName,
+              variablesType: variablesName,
+              api: API,
+            },
+            *fix(fixer) {
+              if (
+                !importDeclaration.specifiers.find(
+                  (node) => node.type === 'ImportSpecifier' && node.local.name === variablesName,
+                )
+              ) {
+                yield fixer.insertTextAfter(queryImportSpecifier, `, ${variablesName}`);
+              }
+              yield fixer.insertTextAfter(queryType, `, ${variablesName}`);
+            },
+          });
+        }
       },
-      type: 'problem',
-      schema: [],
+    };
+  },
+  name: 'missing-graphql-variables-type',
+  meta: {
+    fixable: true,
+    docs: {
+      description: 'useQuery is missing QueryVariables parameter.',
+      recommended: 'error',
     },
-    defaultOptions: [],
-  }),
-};
+    messages: {
+      'missing-graphql-variables-type':
+        '`{{api}}<{{queryType}}>(...)` should be `{{api}}<{{queryType}},{{variablesType}}>(...)`.',
+    },
+    type: 'problem',
+    schema: [],
+  },
+  defaultOptions: [],
+});

js_modules/dagster-ui/packages/eslint-config/rules/no-apollo-client.js

@@ -1,21 +1,12 @@
-/* eslint-disable */
+const {findRelativeImportPath} = require('../util/findRelativeImportPath');
+const {ESLintUtils, AST_NODE_TYPES} = require('@typescript-eslint/utils');
+const createRule = ESLintUtils.RuleCreator((name) => name);
 
-const fs = require('fs');
-const path = require('path');
-
-module.exports = {
-  meta: {
-    type: 'suggestion',
-    fixable: 'code',
-    messages: {
-      useWrappedApolloClient:
-        'Please use our wrapped apollo-client module which includes performance instrumentation.',
-    },
-  },
+module.exports = createRule({
   create(context) {
     return {
-      ImportDeclaration(node) {
-        if (context.getFilename().endsWith('/apollo-client')) {
+      [AST_NODE_TYPES.ImportDeclaration](node) {
+        if (context.getFilename().endsWith('/apollo-client/index.tsx')) {
           return;
         }
         if (node.source.value === '@apollo/client') {
@@ -26,35 +17,29 @@ module.exports = {
               const currentFilePath = context.getFilename();
               const relativeImportPath = findRelativeImportPath(
                 currentFilePath,
-                'src/apollo-client',
-                'index.tsx',
+                'src/apollo-client/index.tsx',
               );
 
-              return fixer.replaceText(node.source, `'${relativeImportPath}'`);
+              // If we can't find the file then it means we're not in the ui-core package (we might be in cloud) so
+              // grab the import from @dagster-io/ui-core.
+              return fixer.replaceText(
+                node.source,
+                `'${relativeImportPath ?? '@dagster-io/ui-core/apollo-client'}'`,
+              );
             },
           });
         }
       },
     };
   },
-};
-
-function findRelativeImportPath(currentFilePath, targetDirName, targetFileName) {
-  let currentDir = path.dirname(currentFilePath);
-
-  while (currentDir !== path.parse(currentDir).root) {
-    const srcPath = path.join(currentDir, targetDirName);
-    const targetFilePath = path.join(srcPath, targetFileName);
-
-    if (fs.existsSync(targetFilePath)) {
-      return path.relative(path.dirname(currentFilePath), srcPath);
-    }
-
-    currentDir = path.dirname(currentDir);
-  }
-
-  // If we can't find the file then it means we're not in the ui-core package (we might be in cloud) so
-  // grab the import from @dagster-io/ui-core.
-
-  return '@dagster-io/ui-core/apollo-client';
-}
+  name: 'no-apollo-client',
+  meta: {
+    type: 'suggestion',
+    fixable: 'code',
+    messages: {
+      useWrappedApolloClient:
+        'Please use our wrapped apollo-client module which includes performance instrumentation.',
+    },
+  },
+  defaultOptions: [],
+});