The Wayback Machine - https://web.archive.org/web/20220518055625/https://github.com/mongodb-js/mongosh/pull/784
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(shell-api): Generate shell-api metadata from source #784

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

gribnoysup
Copy link
Contributor

@gribnoysup gribnoysup commented Apr 15, 2021

Here's my 🦨 exploration. This PR adds a script that generates metadata from shell-api collecting type info and documentation of all exposed classes and class members in one place. For the PoC I added only very basic error validation for shell method calls to have some kind of example of how we can use it:

image

I think if we decide to keep it, we can polish error messages and also move help logic to be based on this metadata which can provide a better visual based on shell environment. Also VSCode can consume it instead of i18n data for better intellisense suggestions (e.g., deprecated data is available in the metadata)

I excluded the metadata from source as it's a generated file, so if you want to see how the file looks you'll need to pull the branch and run npm run metadata in shell-api package

);

const metadata = await generateMetadata(files, config);

await fs.writeFile(
await promisify(fs.writeFile)(
Copy link
Contributor

@addaleax addaleax Apr 16, 2021

Choose a reason for hiding this comment

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

Just curious, we use fs.promises a lot in mongosh, and the only release lines without fs.promises but with promisify are 8.x & 9.x, which have been EOL for a while, what’s the situation where we can’t use fs.promises?

Copy link
Contributor Author

@gribnoysup gribnoysup Apr 16, 2021

Choose a reason for hiding this comment

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

Ohhh, yeah, you're totally right, we use it a lot and it doesn't fail. I refactored from failing fs/promises import that was added only in 14 directly to using promisify, totally skipped my mind that fs.promises was added before fs/promises. I'll change it to fs.promises as it's definitely a better option here

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