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

[code-infra][RFC] Set displayName on components in dev mode #209

Open
Janpot opened this issue Oct 8, 2024 · 0 comments
Open

[code-infra][RFC] Set displayName on components in dev mode #209

Janpot opened this issue Oct 8, 2024 · 0 comments
Labels
RFC Request For Comments scope: code-infra Specific to the core-infra product

Comments

@Janpot
Copy link
Member

Janpot commented Oct 8, 2024

What's the problem?

Running our tests in vitest I came across this issue. One (Several tests are failing in browser mode) test is failing because an error message is not matching:

  - Expected #3 "The above error occurred in the <ForwardRef(SelectInput)> component" to be included in 
"The above error occurred in the <ForwardRef(SelectInput2)> component:
...

Notice the mismatch between <ForwardRef(SelectInput)> and <ForwardRef(SelectInput2)>. esbuild compiles the source slightly differently, resulting in a different displayName for the component. Besides that, the ForwardRef(...) wrapper is unnecessary information for the user. I propose we set the displayName property to the exported component and turn that error message into

The above error occurred in the <SelectInput> component

regardless of how the compiler transforms or renames the component. We only do this in dev mode to avoid impact on end user bundle size.

What are the requirements?

Error messages in dev mode are deterministic and don't contain unnecessary wrappers in component names.

What are our options?

  1. Manually add
    if (process.env.NODE_ENV !== 'production') {
      SelectInput.displayName = 'SelectInput'
    }
  2. Let the proptypes generation script add the above snippet automatically
  3. ?

Proposed solution

No response

Resources and benchmarks

We're already doing this for wherever we use a context

https://github.com/mui/material-ui/blob/488bdd9c7c24aacafb4f6b00fe28dce18ada5828/packages/mui-material/src/Accordion/AccordionContext.js#L10-L12

Search keywords:

@Janpot Janpot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer RFC Request For Comments labels Oct 8, 2024
@oliviertassinari oliviertassinari changed the title [RFC] [RFC] Set displayName on components in dev mode Oct 8, 2024
@oliviertassinari oliviertassinari added the scope: code-infra Specific to the core-infra product label Oct 8, 2024
@oliviertassinari oliviertassinari changed the title [RFC] Set displayName on components in dev mode [code-infra][RFC] Set displayName on components in dev mode Oct 8, 2024
@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request For Comments scope: code-infra Specific to the core-infra product
Projects
None yet
Development

No branches or pull requests

2 participants