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

Perf: reduce metadata RSC payload #50678

Merged
merged 2 commits into from Jun 2, 2023
Merged

Perf: reduce metadata RSC payload #50678

merged 2 commits into from Jun 2, 2023

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Jun 2, 2023

What

Currently when all the metadata rendered as JSX as server components, all the empty metadata which should be skipped as they're rendered as null in JSX, are still included in the RSC payload. This helps reduce the initial html size.

How

Change the JSX components into function calls, and filter out the nullable component that won't be rendered, then they won't show up in the react tree and be serialized.

Before

self.__next_f.push([1, "5:[[[\"$\",\"meta\",null,{\"charSet\":\"utf-8\"}],null,null,null,null,null,null,null,null,null,null,[\"$\",\"meta\",null,{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}],null,null,null,null,null,null,null,null,null,null,[]],[null,null,null,null],null,null,[null,null,null,null,null],null,null,null,null,null]\n"])

After

self.__next_f.push([1, "5:[[[\"$\",\"meta\",\"charset\",{\"charSet\":\"utf-8\"}],[\"$\",\"meta\",\"viewport:width=device-width, initial-scale=1\",{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}]]]\n"])

Closes NEXT-1232

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team type: next labels Jun 2, 2023
@huozhi huozhi marked this pull request as ready for review June 2, 2023 11:26
@kodiakhq kodiakhq bot merged commit 1b9fea7 into canary Jun 2, 2023
54 of 55 checks passed
@kodiakhq kodiakhq bot deleted the perf-metadata branch June 2, 2023 11:30
Copy link
Contributor

@gnoff gnoff left a comment

Choose a reason for hiding this comment

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

@huozhi This PR makes the case where there is very little metadata and makes it better at the cost of making cases where there is a lot of metadata much worse.

If you look at the flight encoding of each React element you'll see we serialize the key. The way you implemented the MetaFilter component you rely on keys being set to some unique value of the element, often content. in the case of the description meta tag this means we will send the description twice, once as a key and once as the content prop.

I'd recommend we either revert or fast follow with a refactor that more fully implements teh mutable list of tags.

What I was imagining looks something like this

async function MetadataTree() {
  // psuedocode
  let resolvedMetadata = await getResolvedMetadata();
  
  let elements = []
  
  captureGenericMetadata(elements, resolvedMetadata);
  captureOpengraphMetadata(elements, resolvedMetadata);
  //... each capture function pushes a jsx element keyed off the 
  // elements length
  
  return elements
}

function captureGenericMetadata(target, resolvedMetadata) {
  let nextKey = target.length;
  if (resolvedMetadata.description != null) {
    target.push(<meta key={nextKey++} name="description" content={metadata.description} />)
    }
  } 
}

the only reason it's "ok" to use this index as a key is because semantically the entire set of metadata is keyed by the url and so the only time we'd reconcile the inner children we'd invalidate the parent fragment so all children get replaced anyway.

The nice thing here with this stragegy is the key in the flight payload before would have been null and it will be replaced with 0 or 1 etc... this makes the payload even smaller than it was before.

There are additional benefits in that we create only a single array whereas the current strategy allocates many times due to a lot of mapping

@huozhi
Copy link
Member Author

huozhi commented Jun 3, 2023

@gnoff Thanks, filed a follow up PR for it #50739

kodiakhq bot pushed a commit that referenced this pull request Jun 6, 2023
Follow up for #50678 as @gnoff commented in #50678 (review)

Now metadata is shorter with the shorter key with just numbers

```
self.__next_f.push([1,"5:[[\"$\",\"meta\",\"0\",{\"charSet\":\"utf-8\"}],[\"$\",\"meta\",\"1\",{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}]]\n"])
```
hydRAnger pushed a commit to hydRAnger/next.js that referenced this pull request Jun 12, 2023
### What
Currently when all the metadata rendered as JSX as server components, all the empty metadata which should be skipped as they're rendered as `null` in JSX, are still included in the RSC payload. This helps reduce the initial html size.

### How

Change the JSX components into function calls, and filter out the nullable component that won't be rendered, then they won't show up in the react tree and be serialized.

Before
```
self.__next_f.push([1, "5:[[[\"$\",\"meta\",null,{\"charSet\":\"utf-8\"}],null,null,null,null,null,null,null,null,null,null,[\"$\",\"meta\",null,{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}],null,null,null,null,null,null,null,null,null,null,[]],[null,null,null,null],null,null,[null,null,null,null,null],null,null,null,null,null]\n"])
```

After
```
self.__next_f.push([1, "5:[[[\"$\",\"meta\",\"charset\",{\"charSet\":\"utf-8\"}],[\"$\",\"meta\",\"viewport:width=device-width, initial-scale=1\",{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}]]]\n"])
```


Closes NEXT-1232
hydRAnger pushed a commit to hydRAnger/next.js that referenced this pull request Jun 12, 2023
Follow up for vercel#50678 as @gnoff commented in vercel#50678 (review)

Now metadata is shorter with the shorter key with just numbers

```
self.__next_f.push([1,"5:[[\"$\",\"meta\",\"0\",{\"charSet\":\"utf-8\"}],[\"$\",\"meta\",\"1\",{\"name\":\"viewport\",\"content\":\"width=device-width, initial-scale=1\"}]]\n"])
```
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants