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

Add "Added in <version>" note for each part of the stable API #6308

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mkarajohn
Copy link

@mkarajohn mkarajohn commented Sep 21, 2023

This PR introduces a new "Added in <version_number>" section (which links to the relevant changelog) for each part of the API that is currently included in the stable versions.

Only part that gave me some trouble and could not exactly pinpoint when it was added was flushSync which must have been released somewhere during v16 according to this but it only got officially documented in the v18 docs.

If someone knows the exact version this was added I will gladly add it.

image

@github-actions
Copy link

github-actions bot commented Sep 21, 2023

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

Three Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/404 77.6 KB (🟡 +348 B) 181.55 KB
/500 77.59 KB (🟡 +347 B) 181.54 KB
/[[...markdownPath]] 79.14 KB (🟡 +348 B) 183.09 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 10% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

@mkarajohn mkarajohn changed the title Add first version appearance for API parts Add "Added in <version>" note for each part of the stable API Sep 22, 2023
@mkarajohn
Copy link
Author

...any thoughts on this?

@sophiebits
Copy link
Member

Thoughts @mattcarrollcode? I don't think this is quite the right design but the rough idea seems good. There might also be certain parts of the API that were added later. I wonder if something more like the MDN browser compatibility table later in the page would make sense (perhaps comparing RSC/SSR/Client…? not sure).

@mkarajohn
Copy link
Author

mkarajohn commented Oct 17, 2023

I don't think this is quite the right design but the rough idea seems good.

For the record, if the idea ends up being ok'ed, I am fully willing to take it upon me to implement whatever design the team deems fitting 💪

EDIT:
My "inspiration" for the proposed implementation were the lodash docs

@@ -2,6 +2,10 @@
title: flushSync
---

<AddedInVersion version="16" />

[//]: # (According to the linked answer from Dan Abramov, this was supposed to be released during v16, but is only officially documented in v18.0.0 --> https://github.com/facebook/react/issues/11527#issuecomment-360199710)
Copy link
Contributor

@AhmedBaset AhmedBaset Oct 18, 2023

Choose a reason for hiding this comment

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

Remove this and add as a review comment or comment it in mdx

Suggested change
[//]: # (According to the linked answer from Dan Abramov, this was supposed to be released during v16, but is only officially documented in v18.0.0 --> https://github.com/facebook/react/issues/11527#issuecomment-360199710)
{/*(According to the linked answer from Dan Abramov, this was supposed to be released during v16, but is only officially documented in v18.0.0 --> https://github.com/facebook/react/issues/11527#issuecomment-360199710)*/}

or

Suggested change
[//]: # (According to the linked answer from Dan Abramov, this was supposed to be released during v16, but is only officially documented in v18.0.0 --> https://github.com/facebook/react/issues/11527#issuecomment-360199710)

Copy link
Author

Choose a reason for hiding this comment

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

Resolved in 2fbf483

@@ -2,6 +2,10 @@
title: Children
---

<AddedInVersion version="0.10.0" />

[//]: # (Does not show up in changelogs but only appears in v0.10.0 docs based on https://web.archive.org/ research)
Copy link
Contributor

@AhmedBaset AhmedBaset Oct 18, 2023

Choose a reason for hiding this comment

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

Suggested change
[//]: # (Does not show up in changelogs but only appears in v0.10.0 docs based on https://web.archive.org/ research)
{/*(Does not show up in changelogs but only appears in v0.10.0 docs based on https://web.archive.org/ research)*/}
Suggested change
[//]: # (Does not show up in changelogs but only appears in v0.10.0 docs based on https://web.archive.org/ research)

Copy link
Author

@mkarajohn mkarajohn Oct 19, 2023

Choose a reason for hiding this comment

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

Thanks, changed in 2fbf483. The previous syntax was just me ctrl + / in my editor and it adding the syntax for what it thought was an mdx comment.

@mkarajohn
Copy link
Author

mkarajohn commented Nov 8, 2023

BTW, this is one example of why I believe this PR has value. People not being sure since when a particular feature exists, falsely leading them to believe they can't use it
image

Also, not trying to be annoying here 😅, but I would like to know more of your thoughts about this; about its fate. Would you consider adding it? Like I said I am willing to work on any feedback you give if it means it gets merged.

Thanks

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.

4 participants