Need help with new recipe

[nicojs]

Hi 👋

@JakobJingleheimer invited me to contribute to the 'type-annotationify' codemod recipe. I'm looking into how to do that. I have zero experience with codemod. I heard of it for the first time today 😅.

I have some questions:

1. What would the name of the recipe be? Would "type-annotationify" be acceptable?
2. I don't see a compile step to JS. Is that not needed in the codemod registry?
3. How could I reuse the code inside a recipe directly in an NPM package?

Thanks a lot for any help.

[JakobJingleheimer]

Hiya Nico!

Thanks for popping over 🙂

There's some good info in CONTRIBUTING.md

1. Ideally the name is very explicit about what it does. type-annotationify is okay but a bit colloquial. If you're open to suggestions, I think something like transform-types-to-annotations would be better (albeit less fun).
2. Compilation of recipe source-code from TS → JS happens during publication to the codemod registry. That part is nearly finished (chore(ci): add "publish" GHA #11). We discovered a couple small issues that we're polishing before landing that. If you also want to publish to npm, you can do that locally with whatever transpiler you prefer (we get very good results from esbuild) and then npm publish.
3. That's quite feasible (correct-ts-specifiers is runnable via node directly—that's how I tested it again existing projects), so it's quite easy:

   transform-types-to-annotations/
   ├ src/
   ┆ ├ transformers/
   ┆ ┆ ├ enums/
   ┆ ┆ ┆ ├ README.md
   ┆ ┆ ┆ ├ transform-enums.ts
   ┆ ┆ ┆ ├ transform-enums.spec.ts
   ┆ ┆ ┆ └ transform-enums.test.ts
   ┆ ┆ ├ namespaces/
   ┆ ┆ ┆ └ …
   ┆ ┆ ├ parameter-properties/
   ┆ ┆ ┆ └ …
   ┆ ┆ └ type-assertion-expressions/
   ┆ ┆   └ …
   ┆ └ index.ts
   ├ README.md
   ├ codemodrc.json
   └ package.json
   

   • index.ts would contain the "searching" part of your migration (setting up an AST and grabbing the pieces from it that are relevant).
   • index.ts would then call some exports transformers to handle the actual transformation.
   • To keep things clean, it would probably be a good idea to have a dedicated transformer for each kind of transformation.

[nicojs]

Thanks! I've read through the CONTRIBUTING.md, but it assumes a lot of knowledge about codemod that I don't have. I would love some pointers of what codemod is, how a recipe is called and how to run it locally.

1. Indeed, type-annotationify is a bit tongue-in-cheek funny. I can keep that name in my own package and then call the transformations from this recipe. However, transform-types-to-annotations is not strictly correct since a type is not an annotation. Type aliases are left in instead of transformed. In TypeScript, the flag disallowing enum, parameter properties, and namespaces will be called "erasableSyntaxOnly", so maybe we can do something with that. transform-typescript-to-erasable-syntax. Or to keep it closer to Node's wording: transform-typescript-to-strip-types-syntax.
2. If I understand it correctly, the node package would use a different compile step than the coded package. That's fine. In that case, I would probably go with the TypeScript compiler. I would also like to add a prefix or @userland-migrations org or something. I also suggest using the publish-pipeline for this. That way, versions are always in sync with each other.
3. Agreed with that setup. Nice and clean.

Finally, I would like to introduce StrykerJS. Code coverage is fast and handy, but if you really care about test quality, Stryker is your best friend (although I'm, of course, biased as I'm the author).

If you agree with all this than I suggest the following steps:

• 1. I finish up the features in type-annotationify. It allows me to move fast and not be afraid to break things. We can also test it out in real life fast.
• 2. A PR to userland-migrations to get the transformations here with codemod and test coverage, etc.
• 3. A PR to userland-migrations to add a publish-to-npm pipeline
• 4. Remove the implementation from type-annotationify, and make it use userland-migrations underneath
• 5. A PR to userland-migrations to enable StrykerJS in each recipe.

[JakobJingleheimer]

I would love some pointers of what codemod is

https://docs.codemod.com/introduction

Some people from codemod are also on the userland-migrations team.

how a recipe is called and how to run it locally.

https://github.com/nodejs/userland-migrations/blob/main/recipes/correct-ts-specifiers/package.json#L11

1. Indeed, type-annotationify is a bit tongue-in-cheek funny. I can keep that name in my own package and then call the transformations from this recipe.

Actually, I think that's very easy to facilitate:

• package.json name → type-annotationify
• codemodrc.json name → transform-…

However, transform-types-to-annotations is not strictly correct since a type is not an annotation.

Sure, that was just an example ;)

transform-typescript-to-erasable-syntax

Sure :) I used ts instead of typescript, but maybe I should rename mine to typescript 🤔

1. If I understand it correctly, the node package would use a different compile step than the coded package.

Perhaps, perhaps not. codemod publish uses esbuild, so if you use that too, you should get the same output.

That's fine. In that case, I would probably go with the TypeScript compiler.

You can, but the typescript compiler is significantly slower. For small software, that doesn't matter too much.

I would also like to add a prefix or @userland-migrations org or something. I also suggest using the publish-pipeline for this. That way, versions are always in sync with each other.

We don't currently plan to support both codemod and npm registries, but we wouldn't stop (or try to hinder) contributors facilitating the npm option themselves. If we do in future, we would probably use @nodejs like @nodejs/correct-ts-specifiers (if that were the case though, you couldn't have your more fun name for npm—it would need to be the same for both 😞).

Finally, I would like to introduce StrykerJS.

If I understand correctly what Stryker does, it's redundant to what we already have, and unfortunately, there can't be divergent testing (this would be too difficult to support in CI, plus if there is a problem that a collaborator needs to fix, we can't be learning a dozen different test runners just to fix a problem). That is why I offered to help / handle migrating your tests if you need that (offer still stands 🙂). If there's a feature in Stryker that you need that doesn't exist in node's test runner, I'm also on the node test runner team, so perhaps I can get it added.

1. I finish up the features in type-annotationify. It allows me to move fast and not be afraid to break things. We can also test it out in real life fast.
2. A PR to userland-migrations to get the transformations here with codemod and test coverage, etc.
3. A PR to userland-migrations to add a publish-to-npm pipeline
4. Remove the implementation from type-annotationify, and make it use userland-migrations underneath
5. A PR to userland-migrations to enable StrykerJS in each recipe.

1 & 2 sound fine.

3 I think we do not want at this time. @nodejs/userland-migrations thoughts?

4 It may be easier for you to achieve your npm stuff by creating a fork of userland-migrations were you add that on the side (I think in your fork you would probably want to replace .github/workflows/publish.yml with your own that instead publishes to npm).

5 I think this is redundant to the tooling already in place. If not, willing to consider :)

[nicojs]

Ok, I think I understand. I will go ahead with point 1 and report back here when that is done.

If I understand correctly what Stryker does, it's redundant to what we already have

I've introduced Stryker in type-annotationify; you can see the report here: https://dashboard.stryker-mutator.io/reports/github.com/nicojs/type-annotationify/main#mutant.

This is the PR that introduced it: nicojs/type-annotationify#12

To be very clear, StrykerJS is not a test runner. It is a way to verify that your tests catch actual bugs. The report it produces marks potential bugs that your tests are not seeing. It subsumes code coverage and is superior to it.

I want to understand where you got the impression that Stryker is redundant to what you already have. There may be something wrong with our messaging.

[JakobJingleheimer]

Ahh, well that's a horse of a different colour then! Let me look more into it 🙂

I got the impression from their homepage, which said it was mutation testing (which seemed redundant to what a snapshot can do).