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

[data]: Rename types file from .d.ts to .ts #36062

Merged
merged 1 commit into from
Oct 29, 2021

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Oct 28, 2021

Description

While working on #35869 I discovered that the module where we keep some
top-level type definitions is named with the .d.ts suffix as if it
were a manually-maintained types definition file.

It's capable of being a normal TypeScript module though, and my IDE was
having a bit of trouble with the .d.ts suffix - not because that was
wrong, but I think because it was different than what the IDE expected.

In this patch all we're doing is renaming the file in order to smooth
this over and clean up later type-related patches that will occur to
the data package. Updating tsconfig.json was necessary becuase of the
rename so that TypeScript will appropriately find and use these types.

How has this been tested?

There should be no change in the bundled JavaScript code. There should be
no functional or visual changes.

❓ Could this impact any third-party projects dependent on @wordpress/data?
We need to verify that this generates the same type definitions for the package
as it previously did.

Types of changes

Renaming a module holding type definitions.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

While working on #35869 I discovered that the module where we keep some
top-level type definitions is named with the `.d.ts` suffix as if it
were a manually-maintained types definition file.

It's capable of being a normal TypeScript module though, and my IDE was
having a bit of trouble with the `.d.ts` suffix - not because that was
wrong, but I think because it was different than what the IDE expected.

In this patch all we're doing is renaming the file in order to smooth
this over and clean up later type-related patches that will occur to
the data package. Updating `tsconfig.json` was necessary becuase of the
rename so that TypeScript will appropriately find and use these types.
@dmsnell dmsnell requested a review from nerrad as a code owner October 28, 2021 20:57
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

This makes sense, thanks 👍

@@ -17,5 +17,6 @@
"src/redux-store/metadata/**/*",
"src/promise-middleware.js",
"src/utils",
"src/*.ts"
Copy link
Member

Choose a reason for hiding this comment

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

This could probably be src/**/*.ts, I expect TypeScript files anywhere are expected to be included 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll update that when I actually start using .ts files in there. How does that sound?

Copy link
Contributor

@sarayourfriend sarayourfriend left a comment

Choose a reason for hiding this comment

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

LGTM

@sarayourfriend sarayourfriend added [Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality labels Oct 29, 2021
@dmsnell
Copy link
Member Author

dmsnell commented Oct 29, 2021

I've rebuilt the project and it appears like the generated types.d.ts file is identical

@dmsnell dmsnell merged commit 211eebe into trunk Oct 29, 2021
@dmsnell dmsnell deleted the dmsnell/rename-data-types-file branch October 29, 2021 18:59
@github-actions github-actions bot added this to the Gutenberg 11.9 milestone Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants