-
Notifications
You must be signed in to change notification settings - Fork 798
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
fix(compiler): fix transforming method parameters into docs #5166
Conversation
|
Path | Error Count |
---|---|
src/dev-server/index.ts | 37 |
src/mock-doc/serialize-node.ts | 36 |
src/dev-server/server-process.ts | 32 |
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts | 22 |
src/compiler/prerender/prerender-main.ts | 22 |
src/testing/puppeteer/puppeteer-element.ts | 22 |
src/runtime/client-hydrate.ts | 20 |
src/screenshot/connector-base.ts | 19 |
src/runtime/vdom/vdom-render.ts | 18 |
src/compiler/config/test/validate-paths.spec.ts | 16 |
src/dev-server/request-handler.ts | 15 |
src/compiler/prerender/prerender-optimize.ts | 14 |
src/compiler/sys/stencil-sys.ts | 14 |
src/compiler/transpile/transpile-module.ts | 14 |
src/runtime/vdom/vdom-annotations.ts | 14 |
src/sys/node/node-sys.ts | 14 |
src/compiler/prerender/prerender-queue.ts | 13 |
src/compiler/sys/in-memory-fs.ts | 13 |
src/runtime/connected-callback.ts | 13 |
src/runtime/set-value.ts | 13 |
Our most common errors
Typescript Error Code | Count |
---|---|
TS2345 | 376 |
TS2322 | 373 |
TS18048 | 286 |
TS18047 | 102 |
TS2722 | 37 |
TS2532 | 30 |
TS2531 | 23 |
TS2454 | 14 |
TS2352 | 12 |
TS2790 | 10 |
TS2769 | 8 |
TS2538 | 8 |
TS2344 | 6 |
TS2416 | 6 |
TS2493 | 3 |
TS2488 | 2 |
TS18046 | 2 |
TS2684 | 1 |
TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
File | Line | Identifier |
---|---|---|
src/runtime/bootstrap-lazy.ts | 21 | setNonce |
src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
src/testing/testing-utils.ts | 198 | withSilentWarn |
src/utils/index.ts | 145 | CUSTOM |
src/utils/index.ts | 269 | normalize |
src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
src/compiler/app-core/app-data.ts | 25 | BUILD |
src/compiler/app-core/app-data.ts | 115 | Env |
src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
fb5025e
to
32a4c83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried it out locally and I think everything looks good. I also tried out annotating my method parameter with an imported type, where I had
type Bar = 'asdf'
type Baz = "biiii"
export type Foo = (asdf: string) => Bar | Baz;
in a module and my method looks like
/**
* @param active foobar some description
*/
@Method()
async setActive(active: Foo) {
console.log(active);
}
and everything looks good! I get a nice clean table like this in the README under 'Parameters' 🎉🎉🎉
Name | Type | Description |
---|---|---|
active |
(asdf: string) => "asdf" | "biiii" |
foobar some description |
I just had one question about a small possible testing improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I agree with @alicewriteswrongs' comments, but I'll be happy with her ✔️ (I don't need to re-review, but LMK if you'd like me to)
fixes #4394 STENCIL-846
32a4c83
to
9f9cc08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great!
What is the current behavior?
Parsing parameters from a Stencil component currently relies on a JSDoc string and not actually on the types defined by the user in TypeScript. With that most method types were not documented at all.
fixes #4394
STENCIL-846
What is the new behavior?
I updated the part where we propagate parameter informations and now propagate a parameter name and type to the resulting JSON as well as made updates so that these types are correctly set in the markdown docs.
Does this introduce a breaking change?
No changes to any public TS interfaces.
Testing
To test this behavior:
docs-json
output target, e.g.npx stencil build --docs
You will see an updated
src/components/my-component/readme.md
that has missing details about the method parameters:and a
components.json
that has missing information about parameters too:After checking out this branch and install the Stencil version to the project, the readme file now has the correct parameters table:
and the parameters are set correctly in the json file:
Other information