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

[DevTools] Support secondary environment name when it changes #30842

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

sebmarkbage
Copy link
Collaborator

We currently support the Environment Name change within a Component. #29867

If this happens, we give it two HoCs. The problem with this is that we only show one followed by +1 in the list.

Before:
Screenshot 2024-08-28 at 6 50 31 PM

After:
Screenshot 2024-08-28 at 7 16 21 PM

I could potentially instead badge this case as A/B in a single badge.

Copy link

vercel bot commented Aug 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 28, 2024 11:22pm

@@ -2220,6 +2221,9 @@ export function attach(
let displayName = componentInfo.name || '';
if (typeof env === 'string') {
// We model environment as an HoC name for now.
if (secondaryEnv !== null) {
displayName = secondaryEnv + '(' + displayName + ')';

This comment was marked as off-topic.

// If the next environment is different then this component was the boundary
// and it changed before entering the next component. So we assign this
// component a secondary environment.
return componentInfo.env !== debugEntry.env ? debugEntry.env : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

What if componentInfo.env is nullish, but debugEntry.env is not? Like when VirtualInstance is a descendant of FiberInstance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's no instances here. This is just walking the debugInfo array and comparing it to other entires in that array.

componentInfo.env really can't be nullish right now since there's always an environment for a server component. It's mainly just defensive in case we support other types of virtual components in the future. If it differs, then it's a change in environment so should specify the secondary if available.

However, I don't actually use the secondary environment later on if the first one isn't specified. I assume that then it was just a client component or something that was inlined. But that case doesn't really exist yet.

@sebmarkbage sebmarkbage merged commit e56f4ae into facebook:main Aug 30, 2024
184 checks passed
sebmarkbage added a commit that referenced this pull request Sep 3, 2024
Stacked on #30842.

This adds a filter to be able to exclude Components from a certain
environment. Default to Client or Server.

The available options are computed into a dropdown based on the names
that are currently used on the page (or an option that were previously
used). In addition to the hardcoded "Client". Meaning that if you have
Server Components on the page you see "Server" or "Client" as possible
options but it can be anything if there are multiple RSC environments on
the page.

"Client" in this case means Function and Class Components in Fiber -
excluding built-ins.

If a Server Component has two environments (primary and secondary) then
both have to be filtered to exclude it.

We don't show the option at all if there are no Server Components used
in the page to avoid confusing existing users that are just using Client
Components and wouldn't know the difference between Server vs Client.

<img width="815" alt="Screenshot 2024-08-30 at 12 56 42 AM"
src="https://github.com/user-attachments/assets/e06b225a-e85d-4cdc-8707-d4630fede19e">
gnoff pushed a commit to vercel/next.js that referenced this pull request Sep 12, 2024
**breaking change for canary users: Bumps peer dependency of React from
`19.0.0-rc-7771d3a7-20240827` to `19.0.0-rc-94e652d5-20240912`**

[diff
facebook/react@7771d3a7...94e652d5](facebook/react@7771d3a...94e652d)

<details>
<summary>React upstream changes</summary>

- facebook/react#30952
- facebook/react#30950
- facebook/react#30946
- facebook/react#30934
- facebook/react#30947
- facebook/react#30945
- facebook/react#30938
- facebook/react#30936
- facebook/react#30879
- facebook/react#30888
- facebook/react#30931
- facebook/react#30930
- facebook/react#30832
- facebook/react#30929
- facebook/react#30926
- facebook/react#30925
- facebook/react#30905
- facebook/react#30900
- facebook/react#30910
- facebook/react#30906
- facebook/react#30899
- facebook/react#30919
- facebook/react#30708
- facebook/react#30907
- facebook/react#30897
- facebook/react#30896
- facebook/react#30895
- facebook/react#30887
- facebook/react#30889
- facebook/react#30893
- facebook/react#30892
- facebook/react#30891
- facebook/react#30882
- facebook/react#30881
- facebook/react#30870
- facebook/react#30849
- facebook/react#30878
- facebook/react#30865
- facebook/react#30869
- facebook/react#30875
- facebook/react#30800
- facebook/react#30762
- facebook/react#30831
- facebook/react#30866
- facebook/react#30853
- facebook/react#30850
- facebook/react#30847
- facebook/react#30842
- facebook/react#30837
- facebook/react#30848
- facebook/react#30844
- facebook/react#30839
- facebook/react#30802
- facebook/react#30841
- facebook/react#30827
- facebook/react#30826
- facebook/react#30825
- facebook/react#30824
- facebook/react#30840
- facebook/react#30838
- facebook/react#30836
- facebook/react#30819
- facebook/react#30816
- facebook/react#30814
- facebook/react#30813
- facebook/react#30812
- facebook/react#30811

</details>

---------

Co-authored-by: vercel-release-bot <infra+release@vercel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants