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

Proposal codemod metadata and sourcemaps #102

Open
Fuzzyma opened this issue Dec 13, 2024 · 1 comment
Open

Proposal codemod metadata and sourcemaps #102

Fuzzyma opened this issue Dec 13, 2024 · 1 comment

Comments

@Fuzzyma
Copy link

Fuzzyma commented Dec 13, 2024

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
Copy link
Contributor

43081j commented Dec 25, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants