Skip to content

Typescript: Generate proper .d.ts files for Core #2271

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

Merged
merged 16 commits into from
Jun 1, 2022
Merged

Conversation

scottrippey
Copy link
Member

@scottrippey scottrippey commented May 30, 2022

What

  • Adds a "types" script which generates the .d.ts output for our builds.
  • Adds a tsconfig.json for each package (which just extends the base config). This is necessary to set the proper "scope" for each build ... otherwise TypeScript will always compile everything.

Migration Pattern

Currently, each package has a top-level index.d.ts file, which contains type definitions for all exported members.
This is problematic, because when a method signature changes, the type definition must be updated manually.

The best, but hardest, solution is to convert all .js files to .ts. While we do anticipate this being the "end game" for us, it's a lot of work to handle at once.

Instead, here's a way to migrate in steps:

  1. Split the index.d.ts contents into separate files.
    For example, I split the victory-core/src/index.d.ts file into a bunch of files, like victory-accessible-group.d.ts, victory-animation.d.ts, victory-label.d.ts and so on. I moved those files into the matching folder, alongside the matching .js file.
  2. Delete the empty index.d.ts file
  3. Rename index.js to index.ts. This usually doesn't require any changes.

That's it! When we create our builds, all our .d.ts files will be copied to the output. Also, all .ts files will be compiled, and the generated .d.ts files will be in the output as well; especially the index.d.ts.

Testing

Build everything via:

yarn nps build-package-libs
  • Verify the output of the folders has proper .d.ts files, like:

image

  • Verify that the top index.d.ts should be the same as the index.ts, just a bunch of exports, like:

image

  • TODO: Somehow verify that the "published types" are working from a "consumer" perspective?

@scottrippey scottrippey marked this pull request as ready for review May 30, 2022 23:08
Copy link
Contributor

@becca-bailey becca-bailey left a comment

Choose a reason for hiding this comment

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

Looks good to me - these type files are much more manageable when we move them into their respective directories. Do you anticipate any problems with us releasing this as a patch version bump? In other words, moving these types around won't change anything for users importing types from Victory, correct?

@scottrippey
Copy link
Member Author

scottrippey commented May 31, 2022

The main difference right now is that we're not exporting * yet. So the props are not yet being exported. I've fixed this already in the next PR, #2274

@scottrippey scottrippey merged commit d3dc7e8 into main Jun 1, 2022
@scottrippey scottrippey deleted the typescript-dts branch June 1, 2022 02:22
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

Successfully merging this pull request may close these issues.

None yet

2 participants