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

feat: introduce typings #113

Merged
merged 10 commits into from
Jun 1, 2024
Merged

Conversation

wellwelwel
Copy link
Contributor

@wellwelwel wellwelwel commented May 30, 2024

Closes #107.

This PR introduces typings (.d.ts files) to quibble.


I opted for an easier way to generate the types, which is to change all the extensions to .ts and then generate the compilation. This gives me all the declaration types as any.

Then I undo the changes and start working on each type properly.


Notes

  • There are no changes in the main code.
  • I've added a test that compiles the typings, but it's important to note that this test doesn't reflect in the main code.
    • This test is designed to ensure that no compilation breaks for final users due to quibble typings.
  • I used any for external parameters that are outside the quibble's control, so users can freely define their own mocks types (or not).
  • Two development dependencies have been added in order to perform the tests:
    • @types/node
    • typescript

I tested it manually in all scenarios listed in the documentation and also my projects that use quibble, just to be sure 🔬

@wellwelwel wellwelwel changed the title feat: introduces typings feat: introduce typings May 30, 2024
@wellwelwel wellwelwel marked this pull request as ready for review May 30, 2024 13:48
@wellwelwel
Copy link
Contributor Author

wellwelwel commented May 30, 2024

@searls, sorry for the long time.

Please feel free to make suggestions, changes and questions 🙋🏻‍♂️

There are some interesting improvements such as defining types dynamically (for final user), but I've chosen to keep this PR as objective as possible.

@wellwelwel
Copy link
Contributor Author

A simple visual overview

Screen Recording 2024-05-30 at 17 11 07

Copy link
Member

@searls searls left a comment

Choose a reason for hiding this comment

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

My understanding is that the types ought to be able to be committed to the repo as a single index.d.ts without any changes to the dependencies, build, etc. That would be my preference.

.gitignore Outdated Show resolved Hide resolved
@wellwelwel
Copy link
Contributor Author

wellwelwel commented May 31, 2024

My understanding is that the types ought to be able to be committed to the repo as a single index.d.ts without any changes to the dependencies, build, etc. That would be my preference.

@searls, thanks for the clarification.

I've removed all external changes to index.d.ts and kept only typings for exported methods that reaches the end user, instead of creating typings for all internal files.

A change in package.json has been made to ensure that both editors, CommonJS and ES Modules imports retrieve the same type definitions (also for compatibility):

+  "types": "./index.d.ts",
   "exports": {
     ".": {
       "require": "./lib/quibble.js",
       "import": "./lib/quibble.mjs",
+      "types": "./index.d.ts"
     },
     "./package.json": "./package.json"
   },

Could you review it again?

@searls searls merged commit fac450e into testdouble:main Jun 1, 2024
6 checks passed
@searls
Copy link
Member

searls commented Jun 1, 2024

Thanks! If there's an issue with these, I'm sure we'll hear about it

@wellwelwel wellwelwel deleted the introduce-typings branch June 1, 2024 10:04
@wellwelwel
Copy link
Contributor Author

Thanks! If there's an issue with these, I'm sure we'll hear about it

I'm watching the repository for issues and PRs. If something new comes up, I can help if necessary 🙋🏻‍♂️

@pksunkara
Copy link

@searls Can we please have a release since this was merged?

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.

TypeScript Support
3 participants