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

Fix JSDoc in default config #1740

Open
orofbrown opened this issue Jul 26, 2023 · 2 comments
Open

Fix JSDoc in default config #1740

orofbrown opened this issue Jul 26, 2023 · 2 comments

Comments

@orofbrown
Copy link

Purpose: Fix the JSDoc in lib/config/default.js so that editors and IDEs can infer the type from the factory function correctly.

The installation steps for this project mention in step 3 that you should copy and paste a code snippet with a comment referring developers to a file at some relative path, which is confusing (lib/config/default.js... I saw this in my codebase and thought it was referring to some file in our app that got deleted). The proper way to offer documentation on the configuration API would be to either include it in @types or use JSDoc properly (ideally both).

Description

When I try to import the path using a JSDoc import, VS Code shows the type as () => any because of the @returns statement in the doc

Expected Behavior

My editor should be able to infer the return type of the definition function, or the default config options should be fully documented, rather than using the shortcut of @returns {object}.

Troubleshooting or NR Diag results

N/A

Steps to Reproduce

Using JSDoc , import the definition export and create a new @typedef

This shows Definition to have the type () => any

/**
 * @typedef {import('newrelic/lib/config/default').definition} Definition
 */

Then obviously this is just any

/**
 * @typedef {ReturnType<Definition>} NewRelicConfig
 */

Screenshot:
image

Your Environment

Editor: VSCode
Node version: N/A but I see it with v16 and v18
OS: replicated on both macOS and Windows 10
node-newrelic version is 9.15

Additional context

I was able to solve this by removing a single line, so I was hoping someone else could take up this PR, because going through the work to figure out the CLA agent is going to be more work than the proposed fix.

Screenshot:
vscode_screenshot

Thanks

@workato-integration
Copy link

@newrelic-node-agent-team newrelic-node-agent-team added this to Triage Needed: Unprioritized Features in Node.js Engineering Board Jul 26, 2023
@bizob2828 bizob2828 removed the bug label Jul 26, 2023
@bizob2828
Copy link
Member

@orofbrown PRs are welcome. The CLA is just a form you have to fill out, it's required for any external contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Node.js Engineering Board
  
Triage Needed: Unprioritized Features
Development

No branches or pull requests

3 participants