Skip to content

Commit

Permalink
Add DeltaCalculator graph invalidation for package exports (experimen…
Browse files Browse the repository at this point in the history
…tal)

Summary:
Implements invalidation of dependencies (marks all graphs as dirty) whenever a `package.json` file is changed.

The reasoning for this is that:
- A package.json update that changes `"exports"` can change resolutions by entirely swapping out modules.
- "Phantom dependencies" can exist when an `"exports"` subpath redirects an apparent subpath to another target file. If `"exports"` is updated to remove this mapping, any file(s) at the apparent subpath would need to be watched.

**This is not a final approach, but is acceptable while the Package Exports feature is `unstable`**.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D43394192

fbshipit-source-id: 0ba2c393d8e2b78144c5991d5f48eadd6c47204a
  • Loading branch information
huntie authored and facebook-github-bot committed Feb 21, 2023
1 parent f0119a4 commit 036ea36
Show file tree
Hide file tree
Showing 10 changed files with 212 additions and 102 deletions.
26 changes: 15 additions & 11 deletions packages/metro/src/DeltaBundler/DeltaCalculator.js
Expand Up @@ -11,6 +11,7 @@

'use strict';

import path from 'path';
import {Graph} from './Graph';
import type {DeltaResult, Options} from './types.flow';
import type {RootPerfLogger} from 'metro-config';
Expand All @@ -33,7 +34,7 @@ class DeltaCalculator<T> extends EventEmitter {
_deletedFiles: Set<string> = new Set();
_modifiedFiles: Set<string> = new Set();
_addedFiles: Set<string> = new Set();
_hasSymlinkChanges = false;
_requiresReset = false;

_graph: Graph<T>;

Expand Down Expand Up @@ -104,13 +105,13 @@ class DeltaCalculator<T> extends EventEmitter {
this._deletedFiles = new Set();
const addedFiles = this._addedFiles;
this._addedFiles = new Set();
const hasSymlinkChanges = this._hasSymlinkChanges;
this._hasSymlinkChanges = false;
const requiresReset = this._requiresReset;
this._requiresReset = false;

// Revisit all files if changes include symlinks - resolutions may be
// Revisit all files if changes require a graph reset - resolutions may be
// invalidated but we don't yet know which. This should be optimized in the
// future.
if (hasSymlinkChanges) {
if (requiresReset) {
const markModified = (file: string) => {
if (!addedFiles.has(file) && !deletedFiles.has(file)) {
modifiedFiles.add(file);
Expand Down Expand Up @@ -207,10 +208,13 @@ class DeltaCalculator<T> extends EventEmitter {
logger: ?RootPerfLogger,
): mixed => {
debug('Handling %s: %s (type: %s)', type, filePath, metadata.type);
if (metadata.type === 'l') {
this._hasSymlinkChanges = true;
if (
metadata.type === 'l' ||
(this._options.unstable_enablePackageExports &&
filePath.endsWith(path.sep + 'package.json'))
) {
this._requiresReset = true;
this.emit('change', {logger});
return;
}
let state: void | 'deleted' | 'modified' | 'added';
if (this._deletedFiles.has(filePath)) {
Expand Down Expand Up @@ -281,13 +285,13 @@ class DeltaCalculator<T> extends EventEmitter {
// If a file has been deleted, we want to invalidate any other file that
// depends on it, so we can process it and correctly return an error.
deletedFiles.forEach((filePath: string) => {
for (const path of this._graph.getModifiedModulesForDeletedPath(
for (const modifiedModulePath of this._graph.getModifiedModulesForDeletedPath(
filePath,
)) {
// Only mark the inverse dependency as modified if it's not already
// marked as deleted (in that case we can just ignore it).
if (!deletedFiles.has(path)) {
modifiedFiles.add(path);
if (!deletedFiles.has(modifiedModulePath)) {
modifiedFiles.add(modifiedModulePath);
}
}
});
Expand Down
Expand Up @@ -30,6 +30,7 @@ describe('DeltaBundler', () => {

const options = {
unstable_allowRequireContext: false,
unstable_enablePackageExports: false,
experimentalImportBundleSupport: false,
onProgress: null,
resolve: (from: string, to: string) => {
Expand Down
Expand Up @@ -38,6 +38,7 @@ describe('DeltaCalculator + require.context', () => {

const options = {
unstable_allowRequireContext: true,
unstable_enablePackageExports: false,
experimentalImportBundleSupport: false,
onProgress: null,
resolve: (from: string, to: string) => {
Expand Down

0 comments on commit 036ea36

Please sign in to comment.