AG-13294 - warn when bar/column sparkline migration can't be done

ag-grid · Dec 3, 2024 · d74aa13 · d74aa13
1 parent b61f424
commit d74aa13
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 19 deletions.
diff --git a/packages/ast/src/types/transform.ts b/packages/ast/src/types/transform.ts
@@ -46,9 +46,13 @@ export interface FsContext extends TransformContext {
   fs: FsUtils;
 }
 
+export interface LogOptions {
+  forceOutput?: boolean;
+}
+
 export interface AstCliContext extends FsContext {
-  warn(node: NodePath<AstNode> | null, message: string): void;
-  fail(node: NodePath<AstNode> | null, message: string): void;
+  warn(node: NodePath<AstNode> | null, message: string | Error, options?: LogOptions): void;
+  fail(node: NodePath<AstNode> | null, message: string | Error, options?: LogOptions): void;
 }
 
 export type AstTransformResult = {

diff --git a/packages/cli/src/codemods/plugins/jscodeshift/jscodeshift.adapter.ts b/packages/cli/src/codemods/plugins/jscodeshift/jscodeshift.adapter.ts
@@ -1,7 +1,10 @@
 import j, { Collection } from 'jscodeshift';
-import { AstCliContext, AstTransform, NodePath } from '@ag-grid-devtools/ast';
+import { AstCliContext, AstTransform, AstTransformContext, NodePath } from '@ag-grid-devtools/ast';
 
-export type JSCodeShiftTransformer = (root: Collection) => void | any;
+export type JSCodeShiftTransformer = (
+  root: Collection,
+  state?: AstTransformContext<AstCliContext>,
+) => void | any;
 
 // Use https://astexplorer.net/ to iterate on your transformer
 // Parser: Typescript
@@ -18,7 +21,7 @@ export const jsCodeShiftTransform = (
   return (_babel) => ({
     visitor: {
       Program: {
-        exit(path: NodePath) {
+        exit(path: NodePath, state: AstTransformContext<AstCliContext>) {
           const root: Collection<any> = j((path.hub as any).file.ast);
           const getFirstNode = () => root.find(j.Program).get('body', 0).node;
 
@@ -28,7 +31,7 @@ export const jsCodeShiftTransform = (
 
           // transform
           for (const transform of transforms) {
-            transform(root);
+            transform(root, state);
           }
 
           // restore initial comment if any

diff --git a/...forms/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/input.ts b/...forms/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/input.ts
@@ -1,4 +1,5 @@
 // @ts-nocheck
+import { testType } from './test-type';
 const column = {
     cellRendererParams: {
         sparklineOptions: {
@@ -30,3 +31,33 @@ const shouldntChange2 = {
         type: 'asdadasqda'
     }
 }
+const typeVariable = 'column'
+const shouldWarn1 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: typeVariable
+        }
+    }
+}
+const type = 'column'
+const shouldWarn2 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type
+        }
+    }
+}
+const shouldWarn3 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: (() => 'column')()
+        }
+    }
+}
+const shouldWarn4 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: testType
+        }
+    }
+}
diff --git a/...orms/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/output.ts b/...orms/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/output.ts
@@ -1,4 +1,5 @@
 // @ts-nocheck
+import { testType } from './test-type';
 const column = {
     cellRendererParams: {
         sparklineOptions: {
@@ -32,3 +33,33 @@ const shouldntChange2 = {
         type: 'asdadasqda'
     }
 }
+const typeVariable = 'column'
+const shouldWarn1 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: typeVariable
+        }
+    }
+}
+const type = 'column'
+const shouldWarn2 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type
+        }
+    }
+}
+const shouldWarn3 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: (() => 'column')()
+        }
+    }
+}
+const shouldWarn4 = {
+    cellRendererParams: {
+        sparklineOptions: {
+            type: testType
+        }
+    }
+}
diff --git a/...form-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/output.warnings.cjs b/...form-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/output.warnings.cjs
@@ -0,0 +1,6 @@
+module.exports = [
+    new SyntaxError(`The grid cellRendererParams sparklineOption "type" cannot be automatically migrated. Please refer to the migration guide for more details: https://ag-grid.com/javascript-data-grid/upgrading-to-ag-grid-33-0/`),
+    new SyntaxError(`The grid cellRendererParams sparklineOption "type" cannot be automatically migrated. Please refer to the migration guide for more details: https://ag-grid.com/javascript-data-grid/upgrading-to-ag-grid-33-0/`),
+    new SyntaxError(`The grid cellRendererParams sparklineOption "type" cannot be automatically migrated. Please refer to the migration guide for more details: https://ag-grid.com/javascript-data-grid/upgrading-to-ag-grid-33-0/`),
+    new SyntaxError(`The grid cellRendererParams sparklineOption "type" cannot be automatically migrated. Please refer to the migration guide for more details: https://ag-grid.com/javascript-data-grid/upgrading-to-ag-grid-33-0/`)
+];
diff --git a/.../transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/scenario.json b/.../transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/scenario.json
@@ -1,7 +1,8 @@
 {
   "scenario": {
     "input": "input.ts",
-    "output": "output.ts"
+    "output": "output.ts",
+    "warnings": "output.warnings.cjs"
   }
 }
 
diff --git a/...s/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/test-type.ts b/...s/transform-sparklines-options-v33-0/__fixtures__/scenarios/sparkline-column/test-type.ts
@@ -0,0 +1 @@
+export const testType = 'column';
diff --git a/...c/codemods/transforms/transform-sparklines-options-v33-0/transformers/sparkline-column.ts b/...c/codemods/transforms/transform-sparklines-options-v33-0/transformers/sparkline-column.ts
@@ -2,15 +2,26 @@ import j from 'jscodeshift';
 import { JSCodeShiftTransformer } from '../../../plugins/jscodeshift';
 
 // update bar/column types to bar with matching direction
-export const columnToVerticalBarTransform: JSCodeShiftTransformer = (root) =>
+export const columnToVerticalBarTransform: JSCodeShiftTransformer = (root, state) =>
   root
     .find(j.ObjectProperty, { key: { name: 'cellRendererParams' } })
     .find(j.ObjectProperty, { key: { name: 'sparklineOptions' } })
     .find(j.ObjectProperty, { key: { name: 'type' } })
-    .filter((path) => ['bar', 'column'].includes((path.value.value as any).value))
     .forEach((path) => {
-      const existingType = (path.value.value as any).value;
-      const direction = existingType === 'column' ? 'vertical' : 'horizontal';
-      path.replace(j.objectProperty(j.identifier('type'), j.stringLiteral('bar')));
-      path.insertAfter(j.objectProperty(j.identifier('direction'), j.stringLiteral(direction)));
+      const typeValue = path.value.value;
+      const isStringLiteral = j.StringLiteral.check(typeValue);
+      if (isStringLiteral && ['bar', 'column'].includes(typeValue.value)) {
+        const existingType = typeValue.value;
+        const direction = existingType === 'column' ? 'vertical' : 'horizontal';
+        path.replace(j.objectProperty(j.identifier('type'), j.stringLiteral('bar')));
+        path.insertAfter(j.objectProperty(j.identifier('direction'), j.stringLiteral(direction)));
+      } else if (!isStringLiteral) {
+        state?.opts.warn(
+          null,
+          new SyntaxError(
+            `The grid cellRendererParams sparklineOption "type" cannot be automatically migrated. Please refer to the migration guide for more details: https://ag-grid.com/javascript-data-grid/upgrading-to-ag-grid-33-0/`,
+          ),
+          { forceOutput: true },
+        );
+      }
     });
diff --git a/packages/codemod-utils/src/transform/js.ts b/packages/codemod-utils/src/transform/js.ts
@@ -65,13 +65,15 @@ function transformJsFile<S>(
   const transformContext: AstTransformContext<AstCliContext> = {
     filename,
     opts: {
-      warn(node: NodePath<AstNode> | null, message: string) {
+      warn(node: NodePath<AstNode> | null, message: string | Error, options) {
         const error = createSourceCodeError(node, message);
-        uniqueErrors.set(error.message, { error, fatal: false });
+        const messageKey = `${error.message}${options?.forceOutput ? Math.random() : ''}`;
+        uniqueErrors.set(messageKey, { error, fatal: false });
       },
-      fail(node: NodePath<AstNode> | null, message: string) {
+      fail(node: NodePath<AstNode> | null, message: string | Error, options) {
         const error = createSourceCodeError(node, message);
-        uniqueErrors.set(error.message, { error, fatal: true });
+        const messageKey = `${error.message}${options?.forceOutput ? Math.random() : ''}`;
+        uniqueErrors.set(messageKey, { error, fatal: true });
       },
       fs,
       userConfig,
@@ -99,6 +101,9 @@ function transformJsFile<S>(
   };
 }
 
-function createSourceCodeError(node: NodePath<AstNode> | null, message: string): Error {
-  return node ? node.buildCodeFrameError(message) : new SyntaxError(message);
+function createSourceCodeError(node: NodePath<AstNode> | null, message: string | Error): Error {
+  if (typeof message === 'string') {
+    return node ? node.buildCodeFrameError(message) : new SyntaxError(message);
+  }
+  return message;
 }