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

Move to TypeScript #78

Merged
merged 22 commits into from
Dec 26, 2024
Merged

Move to TypeScript #78

merged 22 commits into from
Dec 26, 2024

Conversation

vernak2539
Copy link
Owner

@vernak2539 vernak2539 commented Dec 19, 2024

This PR moves this plugin to use TypeScript. It aims to fix #72 and make the upkeep of types (now jsdoc style + custom declaration files) easier.

TODO:

  • Convert main files
  • Convert tests
  • Build
  • Update package.json
  • Docs

Context in #72

@vernak2539
Copy link
Owner Author

@techfg I think this is most of the way done. Last thing is to sort out the docs, which seem to not be generating correctly. Unsure if it's the differences between tsconfig settings or something else. I've optimised the tsconfig settings for the tsup build

@techfg
Copy link
Contributor

techfg commented Dec 20, 2024

@techfg I think this is most of the way done. Last thing is to sort out the docs, which seem to not be generating correctly. Unsure if it's the differences between tsconfig settings or something else. I've optimised the tsconfig settings for the tsup build

Thanks for putting this together. Added some comments that should get you to where you want to be code & doc wise. Note that there are still some issues with typedoc/typedoc-plugin-markdown/typedoc-plugin-zod so I included a workaround (using @interface) for now. That means that the docs will generate as interfaces instead of types which isn't ideal. However, it allows us to make all the code changes to solve both hover & intellisense leaving only the docs to be "fixed" once the typedoc* plugin issues are fully resolved.

@vernak2539
Copy link
Owner Author

Thanks @techfg! I'm not sure I'm seeing comments anywhere though 😢

@vernak2539
Copy link
Owner Author

@techfg I've gotten a bunch further, but ran out of time for today. I'm not sure the docs are generating right (Options seems to be problematic) unfortunately.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved

/** General options */
export type Options = z.infer<typeof OptionsSchema>;
interface EffectiveOptions extends Options {}
Copy link
Contributor

@techfg techfg Dec 20, 2024

Choose a reason for hiding this comment

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

Couple of things here:

  1. Options should use z.input while EffectiveOptions should use z.infer (which is the same as z.output)
  2. There are still some remaining issues with typedoc/typedoc-plugin-markdown/etc. when using types instead of interfaces. If you change Options and CollectionsSchema to interface and make all the other changes that I recommend, the docs should generate identical to current. However, we want Options to be a type to solve for the hover issue. So, we use the @interface typedoc tag as a workaround for now. This is not ideal as the docs will have Options as an interface and we're going to lose the TOC, etc. but the code itself will be correct. I'm going to update the isuses I have filed with typedoc, etc. to cover a few additional situations that I found just now. Once all those issues are addressed, we can eliminate the @interface workaround.
type OptionsSchemaType = typeof OptionsSchema;
type CollectionConfigSchemaType = typeof CollectionConfigSchema;

/** 
 * Collection specific options 
 * @interface
 */
export type CollectionConfig = z.input<CollectionConfigSchemaType>;

/** 
 * General options 
 * @interface
 */
export type Options = z.input<OptionsSchemaType>;
interface EffectiveOptions extends z.infer<OptionsSchemaType> {};

EDIT: Per my recent comment, the @interface workaround should no longer be required and its inclusion can be ignored. The docs should generate correctly with Options and CollectionConfig as types assuming all the changes in other comments are implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vernak2539 -

Sorry if I caused some confusion with my edit regarding @interface :( The part about needing the @interface tag still applies but the other changes are still needed and seem to have been missed:

  1. Options should use z.input - This ensures that the docs correctly reflect optional properties. Currently, for example, collectionBase does not show as optional.
  2. EffectiveOptions should use z.infer - These are the options after defaults are applied so for options that have a default, even if the user doesn't provide one, the effective options, which we use, have proper types based on presence of defaults.
  3. CollectionConfig should be a type - This ensures that "hover" works.

The final code block should be

/** Collection specific options */
export type CollectionConfig = z.input<CollectionConfigSchemaType>;
type CollectionConfigSchemaType = typeof CollectionConfigSchema;

/** General options */
export type Options = z.input<OptionsSchemaType>;
type OptionsSchemaType = typeof OptionsSchema;

interface EffectiveOptions extends z.infer<OptionsSchemaType> {};

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks! Fixed in #79

tsconfig.json Outdated Show resolved Hide resolved
@techfg
Copy link
Contributor

techfg commented Dec 20, 2024

Thanks @techfg! I'm not sure I'm seeing comments anywhere though 😢

Sorry, forgot to hit the "submit" review button - still a weird nuance of github in IMHO that you have to do that instead of it just posting the comment. Either way, you should see the comments now, let me know if you don't.

I'll take a look at the commits you added today and see if anything pops out but the comments you hopefully see now should get you to where you need to be doc wise.

src/plugin.test.ts Show resolved Hide resolved
techfg
techfg previously requested changes Dec 21, 2024
src/index.ts Outdated Show resolved Hide resolved
@techfg
Copy link
Contributor

techfg commented Dec 21, 2024

@vernak2539 -

Great news! Based on input from Gerrit0/typedoc-plugin-zod#8, I was able to successfully generate the docs using types (instead of interfaces)! I did find another issue typedoc2md/typedoc-plugin-markdown#743, but its resolution shouldn't hold up getting this finished and merged. Once that issue is addressed, we can always update deps and push new docs with a TOC.

I added a few comments to the PR and edit a couple that should hopefully get you to where we want to be docs wise.

Lemme know if you have any issues/questions. Getting close, thank you!

@vernak2539 vernak2539 dismissed techfg’s stale review December 26, 2024 16:11

Changes have been implemented!

@vernak2539 vernak2539 merged commit cf25663 into main Dec 26, 2024
9 checks passed
@vernak2539 vernak2539 deleted the move-to-typescript branch December 26, 2024 16:14
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.

bug: hover & intellisense do not work for Options
2 participants