-
-
Notifications
You must be signed in to change notification settings - Fork 276
[code-infra] Migrate error code extraction to code-infra #2670
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
Conversation
| selector = a; | ||
| } else { | ||
| throw new Error('Missing arguments'); | ||
| throw /* minify-error-disabled */ new Error('Missing arguments'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't really save that much.
commit: |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| case 3: return fn(state, a1, a2, a3); | ||
| default: | ||
| throw new Error('unreachable'); | ||
| throw /* minify-error-disabled */ new Error('unreachable'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
| * ... | ||
| */ | ||
| export default function formatErrorMessage(code: number, ...args: string[]): string { | ||
| const url = new URL(`https://base-ui.com/production-error/${code}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented here #1463
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be this or https://base-ui.com/production-error?code=${code}
| if (process.env.NODE_ENV !== 'production') { | ||
| throw new Error('Base UI: Cannot call an event handler while rendering.'); | ||
| } | ||
| throw new Error('Base UI: Cannot call an event handler while rendering.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NODE_ENV check not needed now with extraction enabled.
1516364 to
10cb971
Compare
Bundle size report
|
d936046 to
2dbc69c
Compare
604f17f to
3e1029c
Compare
3e1029c to
4a7124d
Compare
4a7124d to
187001a
Compare
187001a to
43d8fdd
Compare
| 'use client'; | ||
| import * as React from 'react'; | ||
| import { useSearchParams } from 'next/navigation'; | ||
| import codes from 'docs/src/error-codes.json'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can have basic inline markdown formatting rendering in a followup.
| } | ||
|
|
||
| function assertNotCalled() { | ||
| if (process.env.NODE_ENV !== 'production') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 That's a case we may be able to detect in the babel plugin
Janpot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my end 👍
e6da500 to
0a4803a
Compare
0a4803a to
4e1c986
Compare
|
@mui/base-ui Can I get a review/approval ? |
LukasTy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall.
Great work. 👍
| 1. There hasn't been an update to the semantic meaning of the error message. Error codes need to outlive Base UI versions, so the same code must mean the same thing across versions. | ||
| 2. There hasn't been a change in parameters, no added and no removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this be "hardish" to maintain without automation as time goes by? 🤔
https://deploy-preview-2670--base-ui.netlify.app/production-error?code=1