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 · 4 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.

@Janpot
Copy link
Member

Janpot commented Sep 27, 2024

I'm changing my mind about this. The problem is the fact that we use babel-plugin-macros. That's what makes us import code that can't run untranspiled. Instead, we could turn this into valid javascript and write a babel plugin that extracts error codes and replaces them with minified version.

Improved DX:

  • Unlike the current implementation, the plugin would be entirely optional. If in the future we want to switch compilers, we can keep the plugin and run babel as a post-processing step.
  • The infra is easier to share, we don't need both a code import and a babel plugin, we only need a plugin
  • We can just detect every new Error pattern automatically and minify it, maintainers don't need to know about the infra to be notified their error needs to be minified.
  • We can automatically extract interpolations, the maintainer doesn't need to know about message formatting

input:

throw new Error(`Invalid ${foo} in ${bar}`);

Output

import formatProdError from '@mui/utils/formatProdError'
// ...
throw new Error(process.env.NODE_ENV !== 'production' ? `Invalid ${foo} in ${bar}` : formatProdError(123, foo, bar));

and

./error-codes.json
{
  "123": "Invalid % in %"
}
  • We can error on unminifyable patterns, e.g.
    throw new Error(someObject['error'])
  • We can keep the checks on duplicates and interpolation
  • We can add an ignore comment for errors we don't want to minify

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