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: Generate JSON file with TypeDoc #207

Closed
wants to merge 5 commits into from
Closed

feat: Generate JSON file with TypeDoc #207

wants to merge 5 commits into from

Conversation

ericjeker
Copy link
Contributor

@ericjeker ericjeker commented Jan 18, 2022

I rebased from main as the other branch was too far behind and also because in the meantime I deleted it from my fork to have a clean fork.

Now, here's the catch:

TypeDoc generates documentation only from directly exported types from the entrypoint:
TypeStrong/typedoc#1657

So our sub-classes, like Address, are not included in the documentation except if they are exported from index.ts. Not sure if this is desirable.

There are workarounds, but I did not try that yet: https://github.com/Gerrit0/typedoc-plugin-missing-exports

As in Faker we instantiate all those sub-classes:

readonly address: Address = new Address(this);

We could imagine exporting these instances, so ideally we could then access them doing:

import { Address } from '@faker-js/faker';

const cityName = Address.city();

This is up for discussion.

@netlify
Copy link

netlify bot commented Jan 18, 2022

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: dcad3ac

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61e61f54a819b20007c59a94

😎 Browse the preview: https://deploy-preview-207--vigilant-wescoff-04e480.netlify.app

@ericjeker ericjeker mentioned this pull request Jan 18, 2022
@import-brain import-brain requested a review from a team January 18, 2022 04:28
@import-brain import-brain added the s: pending triage Pending Triage label Jan 18, 2022
@Shinigami92
Copy link
Member

import { Address } from '@faker-js/faker';

const cityName = Address.city();

It's sadly not that simple

faker/src/address.ts

Lines 97 to 114 in a973ee1

city(format?: string | number) {
const formats = [
'{{address.cityPrefix}} {{name.firstName}}{{address.citySuffix}}',
'{{address.cityPrefix}} {{name.firstName}}',
'{{name.firstName}}{{address.citySuffix}}',
'{{name.lastName}}{{address.citySuffix}}',
];
if (!format && this.faker.definitions.address.city_name) {
formats.push('{{address.cityName}}');
}
if (typeof format !== 'number') {
format = this.faker.datatype.number(formats.length - 1);
}
return f(formats[format]);
}

Here you can see that city is relying on definitions and the datetype module.
So you would need to always pass that from outside.

city: this.faker.address.city(),

Also modules like e.g. helpers need e.g. this member function.

Beside that it would break the whole projects compatibility. So we cannot work on that before v7 or later.


For now we should try to find another way to generate docs out of the existing code.
Maybe there is just a configuration or we need a JSDoc "annotation".
Maybe it's also possible to export just the types at index.ts like export type { Address } from './address' 🤔

I will test this out a bit later this day.

@import-brain import-brain added needs rebase There is a merge conflict and removed s: pending triage Pending Triage labels Jan 20, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Jan 21, 2022

We already export the global Faker instance.
Maybe we can define that these sub instances also refer to that.

export const faker: Faker = new Faker({
  locales: require('./locales'),
});

export const address = faker.address;
[...]

Not sure how useful that is though.

@ericjeker
Copy link
Contributor Author

Added a plugin to generate the documentation in markdown. For now I generate it under docs/api/typedoc.

There is a VuePress plugin available to integrate all this with VuePress, but sadly VitePress doesn't support plugins the same way, they integrated plugins in "themes".

I am really not familiar with those projects so can't really help with that.

So we have 2 solutions I guess:

  1. We dump VitePress in favor of VuePress. Which seems reasonable as it works out of the box.
  2. We develop a TypeDoc theme or plugin for VitePress, maybe contributing to @tgreyuk project.

@Shinigami92
Copy link
Member

I (personally) would like to choose the hard / second way here, because we would like to bring and push VitePress and the Vite ecosystem forward.
VuePress 1 seems not in active development anymore and introduce (dev) audit vulnerability dependencies. VuePress 2 is also just in beta for more then half a year.
Knowing that in around one month there will be the new Vue 3 docs and on 7 February 2022 Vue 3 will become the new default, we should really stay with VitePress 😉

All the problems we now find will help potentially thousands other devs to not have these issues in the future.

typedoc.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@ericjeker
Copy link
Contributor Author

I already opened an issue here to see if we can contribute to @tgreyuk repo so more people will profit from this. So far, I was able to integrate some of the generated .md with VitePress but some Markdown are not properly generated (e.g. the tables), or properly sanitized and VitePress doesn't like that.

@@ -45,7 +45,8 @@
"format": "prettier --write .",
"lint": "echo 'TODO eslint'",
"test": "vitest",
"coverage": "vitest --coverage"
"coverage": "vitest --coverage",
"typedoc": "npx esno scripts/typedoc.ts"
Copy link
Member

Choose a reason for hiding this comment

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

You can leave out the npx

Suggested change
"typedoc": "npx esno scripts/typedoc.ts"
"typedoc": "esno scripts/typedoc.ts"

@@ -78,13 +79,18 @@
"prettier": "2.5.1",
"simple-git-hooks": "~2.7.0",
"through2": "2.0.0",
"ts-node": "^10.4.0",
Copy link
Member

Choose a reason for hiding this comment

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

We use esno

Suggested change
"ts-node": "^10.4.0",

Comment on lines +83 to +85
"typedoc": "^0.22.11",
"typedoc-plugin-markdown": "^3.11.12",
"typedoc-plugin-missing-exports": "^0.22.6",
Copy link
Member

Choose a reason for hiding this comment

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

For safetyness we should use ~ ranges

Suggested change
"typedoc": "^0.22.11",
"typedoc-plugin-markdown": "^3.11.12",
"typedoc-plugin-missing-exports": "^0.22.6",
"typedoc": "~0.22.11",
"typedoc-plugin-markdown": "~3.11.12",
"typedoc-plugin-missing-exports": "~0.22.6",

Copy link
Member

Choose a reason for hiding this comment

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

After you have done this, you need to regenerate the pnpm-lock.yaml and commit it
It's also a good idea to rm -Rf node_modules pnpm-lock.yaml before using pnpm install to really generate a fresh lock file

"typescript": "~4.5.4",
"vinyl-buffer": "^1.0.1",
"vinyl-source-stream": "^2.0.0",
"vinyl-transform": "^1.0.0",
"vite": "~2.7.13",
"vitepress": "^0.21.4",
"vitest": "~0.1.24"
"vitest": "~0.1.24",
"vuepress-plugin-typedoc": "^0.10.2"
Copy link
Member

Choose a reason for hiding this comment

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

I assume this will not work with vitepress?

As you wrote in the comments, we may need to come up with something on our own or find an equivalent
Or just automate the files our self

Maybe talk to @ST-DDT, he is doing currently really nice automation scripts 🚀

Copy link
Member

Choose a reason for hiding this comment

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

Sure, just tell me what needs to be automated.

@ST-DDT
Copy link
Member

ST-DDT commented Jan 24, 2022

I continued this PR in #289.

@ericjeker
Copy link
Contributor Author

This can be closed and will continue here: #289

@ericjeker ericjeker closed this Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase There is a merge conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants