Review all FatalError throws · facebookarchive/prepack#2020

2018-05-24T20:52:33.000Z

I've seen a few code snippets like this: ```js // todo: emit a diagnostic messsage throw new FatalError(); ``` This is problematic for two reasons: 1. User doesn't get any useful feedback. 2. Worse, on the command-line it will show up as an internal error, because there must not be a `FatalError` without a `CompilerDiagnostic` thingy having been issued. Concrete work items: 1. Review all places in the code where a `FatalError` is thrown, and make sure that a corresponding `CompilerDiagnostic` thingy was issued before. 2. Rethink best practices, as it is too easy to create mismatches between `FatalError` and `CompilerDiagnostic` (how did this get past code review?).

(2 Kommentare) (2 Reaktionen) (0 zugewiesene Personen)JavaScript (14.268 Stars) (520 Forks)batch import

bughelp wanted

Beschreibung

I've seen a few code snippets like this:

    // todo: emit a diagnostic messsage
    throw new FatalError();

This is problematic for two reasons:

User doesn't get any useful feedback.
Worse, on the command-line it will show up as an internal error, because there must not be a FatalError without a CompilerDiagnostic thingy having been issued.

Concrete work items:

Review all places in the code where a FatalError is thrown, and make sure that a corresponding CompilerDiagnostic thingy was issued before.
Rethink best practices, as it is too easy to create mismatches between FatalError and CompilerDiagnostic (how did this get past code review?).

Contributor Guide

Tech Stack: javascript
Domain: developer experiencetooling
Issue Type: refactor
Schwierigkeit: 3
Geschätzte Zeit: half day
Aktivitätsstatus: stale
Klarheit: mostly clear
Voraussetzungen: Understanding of FatalError and CompilerDiagnostic in prepack
Einsteigerfreundlichkeit: 30
Research-Richtung: Review all occurrences of `throw new FatalError()` in the prepack codebase. For each, check if a `CompilerDiagnostic` is emitted before the throw. The issue mentions that some places have a TODO comment without a diagnostic. Look at the linked PRs or comments for any prior discussion. Files likely include compiler error handling modules.