-
Notifications
You must be signed in to change notification settings - Fork 49.8k
[compiler] Migrate CompilerError.invariant to new CompilerDiagnostic infra #34403
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
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.
My only qualm about this is that it makes it a (too?) easy to skip the effort of supplying a proper message on error details. An alternative would be to add an error variant that's just meant for invariants, but that's probably not worth the hassle.
Hmm interesting, my thought was actually the opposite. I was thinking that having the |
With #34176 we now have granular lint rules created for each compiler ErrorCategory. However, we had remnants of our old error severities still in use which makes reporting errors quite clunky. Previously you would need to specify both a category and severity which often ended up being the same. This PR moves severity definition into our rules which are generated from our categories. For now I decided to defer "upgrading" categories from a simple string to a sum type since we are only using severities to map errors to eslint severity. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401). * #34409 * #34404 * #34403 * #34402 * __->__ #34401
Now that we have a new CompilerDiagnostic type (which the CompilerError aggregate can hold), the old CompilerErrorDetail type can be marked as deprecated. Eventually we should migrate everything to the new CompilerDiagnostic type. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34402). * #34409 * #34404 * #34403 * __->__ #34402 * #34401
…infra Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time.
With #34176 we now have granular lint rules created for each compiler ErrorCategory. However, we had remnants of our old error severities still in use which makes reporting errors quite clunky. Previously you would need to specify both a category and severity which often ended up being the same. This PR moves severity definition into our rules which are generated from our categories. For now I decided to defer "upgrading" categories from a simple string to a sum type since we are only using severities to map errors to eslint severity. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401). * #34409 * #34404 * #34403 * #34402 * __->__ #34401 DiffTrain build for [60d9b97](60d9b97)
With #34176 we now have granular lint rules created for each compiler ErrorCategory. However, we had remnants of our old error severities still in use which makes reporting errors quite clunky. Previously you would need to specify both a category and severity which often ended up being the same. This PR moves severity definition into our rules which are generated from our categories. For now I decided to defer "upgrading" categories from a simple string to a sum type since we are only using severities to map errors to eslint severity. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401). * #34409 * #34404 * #34403 * #34402 * __->__ #34401 DiffTrain build for [60d9b97](60d9b97)
…infra (#34403) Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34403). * #34409 * #34404 * __->__ #34403 DiffTrain build for [474f258](474f258)
…infra (#34403) Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34403). * #34409 * #34404 * __->__ #34403 DiffTrain build for [474f258](474f258)
Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time.
Stack created with Sapling. Best reviewed with ReviewStack.