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

docs: fix JSDoc field for object parameter templateParams in Generator constructor #999

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

imballinst
Copy link
Contributor

@imballinst imballinst commented Jun 27, 2023

Description

Previously, we're using String as the field type of templateParams, which was not correct, since we're bound to use Object.keys() on it and Object.keys() on string results on an array over the characters.

The different behaviors between Object<string, string>, Object, and String can be seen in this sandbox: https://codesandbox.io/s/bold-cache-r4tw43?file=/src/index.js.

Initially I wanted to go with just Object (as I mentioned on the issue), but apparently it accepts all kind of types, including string and numbers (which is wrong), so I went with Record<string, string>. Then, I saw the other object field types in the constructor and they are all using Object<string, T>, so I used that instead for consistency purposes.

* @param {Object<String, Boolean | String | String[]>} [options.disabledHooks] Object with hooks to disable. The key is a hook type. If key has "true" value, then the generator skips all hooks from the given type. If the value associated with a key is a string with the name of a single hook, then the generator skips only this single hook name. If the value associated with a key is an array of strings, then the generator skips only hooks from the array.

As we could see from the sandbox, the result is as the following:

/**
 *
 * @param {Object<string, string>} param
 */
function testFnObjectWithTypes(param) {}

testFnObjectWithTypes(123); // TypeError
testFnObjectWithTypes(""); // TypeError
testFnObjectWithTypes({}); // ok
testFnObjectWithTypes({ hello: "world" }); // ok

Related issue(s)

Fixes #985.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@sonarcloud
Copy link

sonarcloud bot commented Jun 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg derberg changed the title fix(generator): fix JSDoc field for object parameter templateParams in Generator constructor docs: fix JSDoc field for object parameter templateParams in Generator constructor Jun 28, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

lgtm @imballinst no need for fix prefix and the release as we are still not exposing any ts interfaces or whatsoever, so just docs need update.

@derberg
Copy link
Member

derberg commented Jun 28, 2023

/rtm

@Florence-Njeri fyi, as docs related. As it is docs, there will an automated PR to update API docs

@asyncapi-bot asyncapi-bot merged commit e3b731b into asyncapi:master Jun 28, 2023
35 checks passed
@imballinst imballinst deleted the fix-generator-typing branch June 28, 2023 15:31
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.10.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Docs Bug 🐞 report]: templateParams should be an object instead of string in the JSDoc
3 participants