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

[WIP] Update Type Definitions and API Docs to Make Sure They Match #5

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

mdemichele
Copy link
Owner

@mdemichele mdemichele commented Feb 18, 2025

I'm trying to go through the Type Definitions and API docs to make sure that they're up to date and fully documented. So far, here are the ones I've found:

  • Adds missing deserialize property to PersistConfig type interface. Updates the APIs docs accordingly. Mentioned in original repo here: #1443

@mdemichele mdemichele changed the title [WIP] Update API docs to reflect correct types [WIP] Update Type Definitions and API Docs to Make Sure They Match Feb 19, 2025
@mdemichele
Copy link
Owner Author

mdemichele commented Feb 20, 2025

Apologies if these are dumb questions, but since I'm not fully an expert in typescript, I need to be honest about gaps in my knowledge and ask a few questions:

  • In the old repo, I saw that the /types folder was added to the .gitignore. But, the older maintainers still left the existing /types folder in the repo. If we're not going to continuing keeping track of changes in the /types folder, shouldn't it be removed from the repo completely? From what I can gather about typescript, it seems like the types.d.ts file inside of the /types folder was auto-generated from the Typescript compiler based off the settings in tsconfig.json and any .ts files the compiler found in the /src folder. If that's the case, then there's not really a reason to keep the contents of the /types folder around. Am I right in that understanding?
  • There wasn't really much of a review process before the original PR went in converting everything to TypeScript. It'd probably be a good idea to revisit that change and make sure everything is set up properly. A couple of questions I have about that PR are: Is there anything in the tsconfig file that we need to change? Are there any places in the /src folder that haven't been fully converted to TypeScript yet? If not, this PR would be a good place to find those leftover things that still need conversion to typescript.


### `type MigrationManifest`
```js
{
[number]: (State) => State
[string]: (State) => State
Copy link
Owner Author

Choose a reason for hiding this comment

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

I changed this to a string type to match what's in the types.ts file, but I'm wondering, is number correct here? If you look at the migrations.md example, the keys are numbers. So, is the types.ts file wrong or is the api.md and migrations.md files wrong?

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.

1 participant