-
Notifications
You must be signed in to change notification settings - Fork 48
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
don't omit message in production #172
base: master
Are you sure you want to change the base?
Conversation
I agree, but I think this would be a breaking change for all the other systems using this. Perhaps, adding an optional parameter to decide whether or not to hide error messages will be helpful, such as: function invariant(condition, message, { hideErrorInfo = process.env.NODE_ENV === "production" } = {}) {
if (hideErrorInfo) {
throw new Error(prefix);
}
} This will ensure that the current interface of invariant(condition, message, { hideErrorInfo: false }) will ensure the actual error is thrown when necessary. |
What do you think about a |
Also would like this PR since in sentry for production builds, I'm only seeing "Invariant failed" which doesn't help with developer debugging. |
@BrianHung Yup, that's exactly my issue as well. Makes it harder to track down what went wrong. |
@alexreardon curious if you have any thoughts here! Would love to help get this improvement in. |
This explains a lot of frustration. Please merge. |
@mykeels what do you think about this? |
Yes please |
I've released my own version of this library with some improvements: https://github.com/epicweb-dev/invariant |
I am open to this. I think that different folks have different needs. For a lot of frontend usages, the ideal is to ship as little code as possible. For backends, having verbose error messages is ideal. Also, some clients might want the verbose messages. I'm open to ideas on how best to solve these needs for various consumers needs in a way the plays nicely with bundlers |
Maybe the answer is not to be clever, but have one export for each variant. Could even use an entry point for extreme bundle size safety import invariant from "tiny-invariant/keep-messages" (For example) |
I join the discussion here. I had to spend some time wondering why my message is not printing in production with only a generic error message. |
It was very unexpected that the production version of the library is different than the development version.
It's very helpful to have the exact invariant message in the exception for easy debugging through tools like sentry.
Curious your thoughts here! I can fix up the tests if you are open to making this change.