Proposal codemod metadata and sourcemaps

[Fuzzyma]

This proposal is based on the discussion in #90.

There are 2 problems right now:

• There is no way to know, if the codemod replaced package imports or similar.
• Sourcemaps also cant be generated

Atm, every codemod is defined as

interface Codemod {
    name: string;
    transform: (options: { file: File }) => string | Promise<string>
}

So, you could probably just add a new field replacements that holds a list with the packages the codemod uses.

interface Codemod {
    name: string;
    replacements: ['picoquery']
    transform: (options: { file: File }) => string | Promise<string>
}

However, this is static and has the obious flaw, that the consumer of the codemod always assumes that the packages in replacements are now in use and should be installed.
It doesnt matter if the codemod didnt do any replacements in the end (because maybe it didnt find an import to replace).

Therefore, it is more useful to return a list of replaced packages from the transform function.
This has the added benefit of solving the second problem: sourcemaps.

Solution

I propose the following interface:

interface CodemodReturn {
    code: string;
    map?: SourceMapV3 | null;
    meta?: {
        replacements?: Record<string, string>;
        warnings?: MaybeLineNumbersAndMessage;
        [key: string]: any;
    }
}

interface Codemod {
    name: string;
    transform: (options: { file: File }) => string | CodemodReturn | Promise<string | CodemodReturn>
}

• code contains the modded code
• map contains a sourcemap (of which type is to be discussed)
• meta can contain any data the codemod wants to return. This should be documented by the codemod. However, some special keys should be reserved:
  • replacements is a list of packages that were replaced by the codemod. This is an object with the form { [oldPackage]: newPackage }. This way, we can track what was replaced by what in case the codemap replaces multiple things. Maybe you want to run another codemod if the first couldnt replace some packages.
  • warnings is an array of warnings. At best, we should have a Codeframe here so we can exactly tell what is a problem where. At least, we should have line numbers and file names.

We might also want to consider adding errors for the case, that the codemod encountered a critical error.
Imho, codemods shouldnt throw and break the pipeline. Instead, they should return the untransformed code with an error message.
An alternative idea would be to use logs that can hold logs of different severity levels. That way we could have all logs, warnings and errors in one property

Compatibility

The former interface must still be supported. So for newer consumers of all codemods, we should declare the rule, that a codemod that returns a string, is to be treated as { code: string }.

Requirements for Codemos

• If all your codemod is doing, is changing some syntax, and you dont want to support sourcemaps, you probably can use the old api.
  This is, to be able to accept new codemods easier which can be refined later. Its always good to have something.
• Codemods that replace imports must be required to list them in replacements. If no replacement happened, it shouldnt be in replacements
• Codemods thar replace imports must support import AND require. PRs that only include one of them should be guided to also implement the other

[43081j]

Do you have an example of when a codemod would want to return arbitrary metadata? I'm not the biggest fan of a bag of properties. So if we can avoid it until we actually need it, I think that'd be better

Even then, I'd probably separate arbitrary key value pairs from known metadata (two objects)

Similarly, source maps sound like a sensible idea but i would leave them out of the initial type until we actually get around to building that functionality. As it will likely develop as we build

[Fuzzyma]

Ok, I agree that airbitrary fields are better hidden away unter its own thing. I left them out for now and made warnings and replacements a first class citizen.
Still thinking about the warnings vs log field. Maybe it would be quite beneficial when codemods would have more possbility to log. Instead of letting the codemod push warnings, we could also pass in a context with debug, log, warn, error methods. That way, we could live log instead of collecting all logs and print them out afterwards. Wdyt?

That said, I did my first implementation with the following api:

import { type SourceMap } from "@ampproject/remapping";
import {
  type Codemod as CodemodV1,
  type CodemodOptions as CodeModeOptionsV1,
} from "module-replacements-codemods";

export { type CodemodV1, type CodeModeOptionsV1 };

export interface Pos {
  /** line number starting from 0 */
  line: number;
  /** column number starting from 0 */
  column: number;
  /** byte offset of the position */
  index: number;
}

export interface Range {
  /** starting position of the range */
  start: Pos;
  /** ending position of the range */
  end: Pos;
}

/**
 * Used to show warnings with links to relevant lines
 */
export interface Warning {
  range?: Range;
  file?: string;
  message: string;
  stack?: string;
}

export interface CodemodMeta {
  /**
   * @example { "lodash": "lodash-es" }
   */
  replacements?: Record<string, string>;
  warnings?: Warning[];
}

export interface TransformResult extends CodemodMeta {
  map?: SourceMap;
  code: string;
}

export interface CodemodOptionsV2 extends CodeModeOptionsV1 {
  strict?: boolean;
}

export interface File {
  source: string;
  filename: string;
}

/**
 * Backwards compatible type for codemods
 * transforms that return a string will treat this string as { code }
 */
export interface CodemodV2 {
  name: string;
  transform: (options: {
    file: File;
  }) => string | Promise<string> | TransformResult | Promise<TransformResult>;
}

I still think that reserving the map field makes sense because if the codemod already produces a sourcemap it would be nicer to not require an update for the codemod just because we left the map field out at the start

[43081j]

i think we should split it up into a few changes:

1. replacements (maybe call it moduleReplacements or something to be clear it is replacements of modules/imports)
2. warnings (maybe we introduce this once we have a codemod which produces warnings. and we may need errors)
3. sourceMap (the most complicated one but useful at some point)

[Fuzzyma]

1. makes sense
2. the qs codemod from the PR i created uses warnings. I used that as an example. There is at least one more taht generates warnings right now. Yes, we might need errors. I proposed to either use "logs" that have warnings and errors and logs or directly pass in functions for logging that the codemod can use (that would be then realtime and maybe better)
3. sourceMap is really just a field now and I would rather already add it since jscodeshift for example will create it if you just pass in the right options on transform.

[43081j]

i think it makes more sense to be very explicit about the warnings and errors rather than treating it like a logger

it isn't a logger, rather it is a strongly typed way to emit errors and warnings (which may or may not be logged by the consumer)

[Fuzzyma]

Well the type would have been: type Log = Warning | Error | Info so it would be typed nonetheless.

But maybe we should go with the idea of passing in log functions. Wdyt about that?

[43081j]

what i mean is its not semantically a log, its more like a result. similar to how eslint works, it returns a result with warnings

a log would be information/telemetry rather than a "result"

so i'd lean more towards having warnings than a generic log

[Fuzzyma]

ok, so the way it is right now. Perfectly fine for me :)