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(nuxt): add @since annotations to exported composables #25086

Merged
merged 26 commits into from Jan 19, 2024

Conversation

luc122c
Copy link
Contributor

@luc122c luc122c commented Jan 6, 2024

πŸ”— Linked issue

#24950

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR is a first step, adding @since annotations to exported composables.

What is @since?

As specified by JSDoc

The @since tag indicates that a class, method, or other symbol was added in a specific version.

Methodology

  • Look for exported functions
  • Git blame to see when the function was created
  • Look for an associated PR / Milestone / Release
  • If the code came across from Nuxt 2 or a Release Candidate, use 3.0.0

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

Copy link

stackblitz bot commented Jan 6, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@DamianGlowala
Copy link
Member

I am wondering whether we should inform about 3.0.0, which is likely to be assumed as the default.

@luc122c
Copy link
Contributor Author

luc122c commented Jan 7, 2024

@DamianGlowala I thought about this myself. I opted to add them 1) so that they would show up in IntelliSense and 2) for completeness. I'm happy to be overruled by the Nuxt team though πŸ‘

@danielroe
Copy link
Member

I'm happy to have 3.0.0 for the record (then, if we ever omit one, it means for sure that we need to add a @since rather than that it's definitely present since 3.0.0)... (More lint-friendly.)

@luc122c
Copy link
Contributor Author

luc122c commented Jan 18, 2024

AI just blew my mind. I asked Phind:

I want to find when this function was added to the codebase

It replied:

To find out when a specific function was added to the codebase, you can use the git blame command followed by the filename. This command shows what revision and author last modified each line of a file.
However, since you're looking for the addition of a specific function, you might want to use the git log command with the -S option, which allows you to search for changes in the amount of occurrences of a string in the source code.
In your case, you can run the following command in your terminal:

git log -S'preloadRouteComponents' -- ./packages/nuxt/src/app/composables/preload.ts

This command will show the commit history where the string 'preloadRouteComponents' appears in the specified file. The first appearance of this string would likely be the addition of the function.

And in the terminal:

% git log -S'preloadRouteComponents' -- ./packages/nuxt/src/app/composables/preload.ts
commit 2a4ebfb18bdffdf9f61e613a2706bc83c4b277c8
Author: Daniel Roe <daniel@roe.dev>
Date:   Mon Oct 17 12:15:29 2022 +0100

    perf(nuxt): improve link prefetching (#8225)

Which led me to nuxt/framework#8225 which was included in 3.0.0-rc.12

@luc122c luc122c marked this pull request as ready for review January 18, 2024 17:14
@luc122c
Copy link
Contributor Author

luc122c commented Jan 18, 2024

All composables exported from packages/nuxt/src/app/composables/index.ts now have a @since annotation.

export function useRequestFetch (): typeof global.$fetch {
if (import.meta.client) {
return globalThis.$fetch
}
return useRequestEvent()?.$fetch as typeof globalThis.$fetch || globalThis.$fetch
}

/** @since 3.0.0 */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When a function has multiple overloads, does each one need it's own annotation? I see that the last overload has a @deprecated annotation which does not affect the first.
Also, what about the case where a new overload is added in a newer version, can that have it's own @since annotation?

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to stick with just one for now.

export function useRequestFetch (): typeof global.$fetch {
if (import.meta.client) {
return globalThis.$fetch
}
return useRequestEvent()?.$fetch as typeof globalThis.$fetch || globalThis.$fetch
}

/** @since 3.0.0 */
Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to stick with just one for now.

packages/nuxt/src/app/composables/fetch.ts Outdated Show resolved Hide resolved
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

Thank you! ❀️

@danielroe danielroe merged commit ce8a2aa into nuxt:main Jan 19, 2024
34 checks passed
@github-actions github-actions bot mentioned this pull request Jan 19, 2024
@luc122c luc122c deleted the docs/add-since branch January 20, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants