Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(hooks): throw invariant violations in production #3217

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

francoischalifour
Copy link
Member

Our invariant util used to throw with a specified error message in development only, and keep the flow going without throwing in production. This is a dangerous behavior to have different flows in development and in production.

This diff now also throws in production, but with a tiny generic error message to keep the bundle size lite. In the future, we'll be able to implement our error strategy with error redirects (similar to React or Next.js) in this invariant.

For reference, tiny-invariant and other projects throw in both environments, only the error messages differ.

I added the production path in the test so that we're able to test that invariant acts as expected in production. __DEV__ is true by default when running the test, but you can run the tests in production with the following command: NODE_ENV=production yarn test invariant

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4c8aaef:

Sandbox Source
react-instantsearch-app Configuration
routing-basic Configuration
algolia/react-instantsearch Configuration

@netlify
Copy link

netlify bot commented Nov 30, 2021

✔️ Deploy Preview for react-instantsearch ready!

🔨 Explore the source changes: 4c8aaef

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-instantsearch/deploys/61a5e8904f16030008675691

😎 Browse the preview: https://deploy-preview-3217--react-instantsearch.netlify.app

Comment on lines +14 to +16
if (!__DEV__) {
throw new Error('Invariant failed');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now, but we should improve the DX at some point so users can easily debug production errors. Granted, they can reproduce in dev to get the full error message, but if they're using error reporting services like Sentry which stack errors and labels them by error message, it's really convenient to be able to find the reason right away. Also useful in difficult to reproduce situations.

I like how React does dit it (the decoder no longer seem to work). We could have good 'ol error numbers associated with invariants and have a comprehensive list documented with the full message, explanations of what went wrong, and possible leads to solve the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's mentioned in the description, but I indeed agree that the least we can do is have error codes, but a slightly more advanced code could collect all invariants and warnings, output them in a markdown or similar and have them accessible. Then the shortened message can include a link to that location.

In algolisearch v3 we actually had this system:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Haroenv I agree, it would be nice to generate them. We could also leverage our alg.li to have shorter links and reduce size even more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, I touched this point in my PR description.

This change is just a more streamlined behavior for both development and production – we used to keep the flow going which is arguably worse because it could either end up generating cryptic errors or worse, end up in inconsistent behaviors that should have errored. This change is a preparation for better error management too.

@francoischalifour francoischalifour merged commit 6d3f99c into master Nov 30, 2021
@francoischalifour francoischalifour deleted the fix/invariant-throw-production branch November 30, 2021 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants