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] Create new Error() minification infrastructure #204

Open
oliviertassinari opened this issue Sep 17, 2024 · 3 comments
Open
Labels
performance priority: low To delay as much as possible umbrella For grouping multiple issues to provide a holistic view

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 17, 2024

Steps to reproduce

Today, the components throw errors in plain strings:

https://github.com/mui/mui-x/blob/dd4447c5d26e578def9a560df8b2bbda2a3ca317/packages/x-tree-view/src/internals/plugins/useTreeViewItems/useTreeViewItems.tsx#L36-L43

those add bundle size to end-users

Context

We should be able to encode them with a key and only bundle this. See mui/material-ui#21214 for prior art on Material UI.

@oliviertassinari oliviertassinari added status: waiting for maintainer These issues haven't been looked at yet by a maintainer performance priority: low To delay as much as possible umbrella For grouping multiple issues to provide a holistic view labels Sep 17, 2024
@oliviertassinari oliviertassinari changed the title [code-infra] Create error minification infrastructure [code-infra] Create new Error() minification infrastructure Sep 17, 2024
@Janpot
Copy link
Member

Janpot commented Sep 17, 2024

The method used in core has the best DX, but fully locks us into babel. If we're going to propagate this, can we think of alternatives that still have acceptable DX, but keep the code portable across build tools and runtimes?

For example, would it be an acceptable DX to just write it manually

throw createMinifiedError(process.env.NODE_ENV === 'production' ? 123 : 'non minified error message', foo, bar)
// createMinifiedError

export function createMinifiedError(msg, ...args) {
  if (process.env.NODE_ENV === 'production') {
    return new Error(format(`minified error #${msg}`, ...args))
  }
  return new Error(format(msg, ...args))
}

(assuming we pair this with a lint rule and an extraction script)

@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 17, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 17, 2024

Maybe a SWC plugin could do this?

On the simpler version, I wonder if people will be able to use it reliably. Mistakes potential:

  • removing old message keys, no, it's only incremental
  • updating error message without updating key, no they should aways be in sync
  • not adding new key when arguments changes, no this would show wrong messages

Maybe it's fine.

@Janpot
Copy link
Member

Janpot commented Sep 18, 2024

Maybe a SWC plugin could do this?

It definitely could, but that would:
a. now lock us into babel + swc. What about esbuild? (vite, tsup), or oxc (rolldown).
b. require us to maintain multiple plugins in multiple languages

On the simpler version, I wonder if people will be able to use it reliably. Mistakes potential:

The idea would be that we still have a script, that uses a babel plugin to extract the messages and update a json file. This script can fail on the cases you mention. We run it locally before pushing and in CI to check for changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance priority: low To delay as much as possible umbrella For grouping multiple issues to provide a holistic view
Projects
None yet
Development

No branches or pull requests

2 participants