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

🚀 Feature: Are the Types Wrong workflow #1633

Closed
2 tasks done
johnnyreilly opened this issue Aug 15, 2024 · 6 comments · Fixed by #1644
Closed
2 tasks done

🚀 Feature: Are the Types Wrong workflow #1633

johnnyreilly opened this issue Aug 15, 2024 · 6 comments · Fixed by #1644
Labels
status: in discussion Not yet ready for implementation or a pull request type: feature New enhancement or request

Comments

@johnnyreilly
Copy link
Collaborator

Bug Report Checklist

Overview

You're probably familiar with @andrewbranch's https://github.com/arethetypeswrong/arethetypeswrong.github.io?tab=readme-ov-file which can be used to ensure the types are correct on a package. I've written about it here: https://johnnyreilly.com/dual-publishing-esm-cjs-modules-with-tsup-and-are-the-types-wrong#linting-your-packagejson-file-with-are-the-types-wrong

We care a lot that the package we produce has good types defined. Perhaps a useful thing would be a dedicated workflow that does something like what my blog does to validate that the types have remained correct.

What do you think?

BTW I have been contemplating writing a GitHub Action to wrap up this behaviour - I think it could be generally useful. Not a requirement though - and something we could migrate to later.

BTW @andrewbranch do you have any thoughts about a dedicated GitHub action for ATTW?

Additional Info

No response

@JoshuaKGoldberg
Copy link
Owner

Ha yeah this has crossed my mind. I'm a big fan of the tool of course, but I'm not convinced it's worth the extra config space + CI action usage for folks using a template like this one. If they're on the template then the types publishing should be set up well for them. Maybe it'd belong as a CI check on create-typescript-app itself, to validate that changes don't mess that up for users?

@JoshuaKGoldberg JoshuaKGoldberg added the status: in discussion Not yet ready for implementation or a pull request label Aug 15, 2024
@johnnyreilly
Copy link
Collaborator Author

I generally don't play with my tsconfig.json / package.json on a project too often. But I do tend do that at some point. At the point I do that, I may well break the types on my package, because I'm clumsy. I'd quite like to have something to take care of making sure I don't mess that up. For my money, given that the project is "create-typescript-app" (i.e. it cares about types) having something like this built in feels like a logical fit. You're right, you'd expect this to fail pretty rarely, and others maybe don't update their tsconfig.json / package.json as I do, so it's probably lower in the risk stakes compared to other checks.

I suspect there's an aspect of preference here; for instance I never use knip etc, which other folk would be very important to some people. Whilst I have great anxiety about my types being out of whack! I quite like the idea of a CI check that protects me from myself.

But yeah, I think I personally would like this check to exist, whether it needs to be in CTA is another thing.

@andrewbranch
Copy link

do you have any thoughts about a dedicated GitHub action for ATTW?

I’m pretty sure I saw that someone made one already 🤔 I’m not against it, but I feel like it doesn’t get any easier than run: npx @arethetypeswrong/cli. What are the benefits of wrapping it in an Action?

@johnnyreilly
Copy link
Collaborator Author

johnnyreilly commented Aug 15, 2024

You make a good point - there's not really any particular benefits of wrapping in an action. Maybe just as good to put something in the README to that effect? "If you want to run this in a pipeline all you need is npx @arethetypeswrong/cli"

JoshuaKGoldberg pushed a commit that referenced this issue Oct 10, 2024
<!-- 👋 Hi, thanks for sending a PR to create-typescript-app! 💖.
Please fill out all fields below and make sure each item is true and [x]
checked.
Otherwise we may not be able to review your PR. -->

## PR Checklist

- [x] Addresses an existing open issue: fixes #1633
- [ ] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/create-typescript-app/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

<!-- Description of what is changed and how the code change does that.
-->
This is a prospective PR following discussion in #1633 to add an `are
the types wrong` workflow that checks CTA itself. Given CTA is ESM only,
we do not check for CJS.

See also
#1633 (comment)
Copy link

🎉 This is included in version v1.73.0 🎉

The release is available on:

Cheers! 📦🚀

@JoshuaKGoldberg
Copy link
Owner

Oh, this was really more of a chore:... ah well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: in discussion Not yet ready for implementation or a pull request type: feature New enhancement or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants