Skip to content

Add TypeScript checking#1589

Merged
westonruter merged 12 commits intotrunkfrom
add/typescript-checking
Oct 16, 2024
Merged

Add TypeScript checking#1589
westonruter merged 12 commits intotrunkfrom
add/typescript-checking

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Oct 15, 2024

Disclaimer: This is my first time setting up TypeScript for a project. So I probably missed some things!

  • Add TypeScript as dependency.
  • Add tsc to the npm scripts.
  • Include both js and ts files in lint-staged.
  • Run npm run tsc in lint-staged. Problem: Passing the files does not work, as TypeScript forces them to be considered TS files and not JS files. See comment.
  • Rename misnamed types.d.ts to types.ts and apply Prettier formatting.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature no milestone PRs that do not have a defined milestone for release labels Oct 15, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 15, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter self-assigned this Oct 15, 2024
@thelovekesh
Copy link
Copy Markdown
Member

Rename misnamed types.d.ts to types.ts.

If the type.ts is purely for type checking then I think we can keep it as .d.ts .

@westonruter
Copy link
Copy Markdown
Member Author

If the type.ts is purely for type checking then I think we can keep it as .d.ts .

Unfortunately then TypeScript won't check it at all when skipLibCheck is enabled, and that is required to prevent TypeScript from checking code in node_modules. So I renamed it to .ts so that TypeScript will check it.

Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter Looks reasonable to me. But I'm not the best person to review this, would be great to get @swissspidy's approval on this.

@westonruter
Copy link
Copy Markdown
Member Author

Also, related to .ts vs .d.ts, it turns out that wp-scripts only lints .js, .jsx, .ts, and .tsx files. As soon as I renamed types.d.ts to types.ts then I got linting errors which could then be automatically fixed with wp-scripts format.

@westonruter westonruter merged commit 865fc21 into trunk Oct 16, 2024
@westonruter westonruter deleted the add/typescript-checking branch October 16, 2024 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature

Projects

Status: Done 😃

Development

Successfully merging this pull request may close these issues.

4 participants