Skip to content

Commit

Permalink
Remove Fast Refresh bail-out in DeltaCalculator on certain errors
Browse files Browse the repository at this point in the history
Summary:
Since D51665495, a transform or resolution error encountered while processing modified files will throw *before any change is committed to `Graph`'s state*, so if `DeltaCalculator` catches an error we can be confident `Graph` is still in a good state.

Previously this might've left a modification partially applied to the graph (I assume that's what's meant by "weird state" in the comment here), so `DeltaCalculator` would attempt to detect that and force a reset update. That creates extra work both for Metro (especially as the next modification might not fix the error, we would re-traverse the whole graph on any change perhaps multiple times), and the client, having a full reset to process. It's no longer necessary.

(Note that in any case this safety net had gaps - it was always possible for a bad update to both add and remove modules such that the overall size didn't change.)

Changelog:
```
**[Fix]:** Don't unnecessarily reset Fast Refresh after recovering from certain transform or resolution errors.
```

Reviewed By: GijsWeterings

Differential Revision: D52193122

fbshipit-source-id: 65516257edb0a2cad79c3346f946ab960b7dfc70
  • Loading branch information
robhogan authored and facebook-github-bot committed Dec 22, 2023
1 parent 1d11554 commit 46b02dc
Showing 1 changed file with 0 additions and 9 deletions.
9 changes: 0 additions & 9 deletions packages/metro/src/DeltaBundler/DeltaCalculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ class DeltaCalculator<T> extends EventEmitter {

let result;

const numDependencies = this._graph.dependencies.size;

try {
result = await this._currentBuildPromise;
} catch (error) {
Expand All @@ -146,13 +144,6 @@ class DeltaCalculator<T> extends EventEmitter {
deletedFiles.forEach((file: string) => this._deletedFiles.add(file));
addedFiles.forEach((file: string) => this._addedFiles.add(file));

// If after an error the number of modules has changed, we could be in
// a weird state. As a safe net we clean the dependency modules to force
// a clean traversal of the graph next time.
if (this._graph.dependencies.size !== numDependencies) {
this._graph.dependencies.clear();
}

throw error;
} finally {
this._currentBuildPromise = null;
Expand Down

0 comments on commit 46b02dc

Please sign in to comment.