Skip to content
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.

Sign up for GitHub

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

Watch mode watches for changes in package.json files used in resolution #44935

Merged
merged 3 commits into from
Jul 13, 2021
Merged

Watch mode watches for changes in package.json files used in resolution #44935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbc38f5
watch mode watches for changes in package.json files used in resolution
weswigham Jul 7, 2021
77465dc
Pathify result of realpath
weswigham Jul 12, 2021
258f1f6
Actually accept pathified baselines
weswigham Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/compiler/moduleNameResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ namespace ts {
export interface PackageJsonInfoCache {
/*@internal*/ getPackageJsonInfo(packageJsonPath: string): PackageJsonInfo | boolean | undefined;
/*@internal*/ setPackageJsonInfo(packageJsonPath: string, info: PackageJsonInfo | boolean): void;
/*@internal*/ entries(): [Path, PackageJsonInfo | boolean][];
clear(): void;
}

Expand Down Expand Up @@ -559,7 +560,7 @@ namespace ts {

function createPackageJsonInfoCache(currentDirectory: string, getCanonicalFileName: (s: string) => string): PackageJsonInfoCache {
let cache: ESMap<Path, PackageJsonInfo | boolean> | undefined;
return { getPackageJsonInfo, setPackageJsonInfo, clear };
return { getPackageJsonInfo, setPackageJsonInfo, clear, entries };
function getPackageJsonInfo(packageJsonPath: string) {
return cache?.get(toPath(packageJsonPath, currentDirectory, getCanonicalFileName));
}
Expand All @@ -569,6 +570,10 @@ namespace ts {
function clear() {
cache = undefined;
}
function entries() {
const iter = cache?.entries();
return iter ? arrayFrom(iter) : [];
}
}

function getOrCreateCache<T>(cacheWithRedirects: CacheWithRedirects<T>, redirectedReference: ResolvedProjectReference | undefined, key: string, create: () => T): T {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@ namespace ts {
getSourceFileByPath,
getSourceFiles: () => files,
getMissingFilePaths: () => missingFilePaths!, // TODO: GH#18217
getModuleResolutionCache: () => moduleResolutionCache,
getFilesByNameMap: () => filesByName,
getCompilerOptions: () => options,
getSyntacticDiagnostics,
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/resolutionCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace ts {
updateTypeRootsWatch(): void;
closeTypeRootsWatch(): void;

getModuleResolutionCache(): ModuleResolutionCache;

clear(): void;
}

Expand Down Expand Up @@ -203,6 +205,7 @@ namespace ts {
const typeRootsWatches = new Map<string, FileWatcher>();

return {
getModuleResolutionCache: () => moduleResolutionCache,
startRecordingFilesWithChangedResolutions,
finishRecordingFilesWithChangedResolutions,
// perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
Expand Down
47 changes: 47 additions & 0 deletions src/compiler/tsbuildPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ namespace ts {
readonly allWatchedInputFiles: ESMap<ResolvedConfigFilePath, ESMap<Path, FileWatcher>>;
readonly allWatchedConfigFiles: ESMap<ResolvedConfigFilePath, FileWatcher>;
readonly allWatchedExtendedConfigFiles: ESMap<Path, SharedExtendedConfigFileWatcher<ResolvedConfigFilePath>>;
readonly allWatchedPackageJsonFiles: ESMap<ResolvedConfigFilePath, ESMap<Path, FileWatcher>>;
readonly lastCachedPackageJsonLookups: ESMap<ResolvedConfigFilePath, readonly (readonly [Path, object | boolean])[] | undefined>;

timerToBuildInvalidatedProject: any;
reportFileChangeDetected: boolean;
Expand Down Expand Up @@ -336,6 +338,8 @@ namespace ts {
allWatchedInputFiles: new Map(),
allWatchedConfigFiles: new Map(),
allWatchedExtendedConfigFiles: new Map(),
allWatchedPackageJsonFiles: new Map(),
lastCachedPackageJsonLookups: new Map(),

timerToBuildInvalidatedProject: undefined,
reportFileChangeDetected: false,
Expand Down Expand Up @@ -498,6 +502,12 @@ namespace ts {
currentProjects,
{ onDeleteValue: existingMap => existingMap.forEach(closeFileWatcher) }
);

mutateMapSkippingNewValues(
state.allWatchedPackageJsonFiles,
currentProjects,
{ onDeleteValue: existingMap => existingMap.forEach(closeFileWatcher) }
);
}
return state.buildOrder = buildOrder;
}
Expand Down Expand Up @@ -861,6 +871,11 @@ namespace ts {
getConfigFileParsingDiagnostics(config),
config.projectReferences
);
state.lastCachedPackageJsonLookups.set(projectPath, state.moduleResolutionCache && map(
Copy link
Member

Choose a reason for hiding this comment

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

Because these package json lookups are calculated when program is built.. If the program was already built and then you change package.json file and invoke tsbuild, the change wont be picked by ts build and it will be reported as upto date.

Also note that ts build uses the infomation from tsconfig to derive if things can be upto date. So we never watch module resolution etc. So this seems one off.
ts build also doesnt mean its incremental build so storing this info in tsbuildinfo is not enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep - I said as much in the OP of this PR - node_modules lookups aren't part of the file set calculated by build mode (seems like an oversight, but also super nontrivial to calculate), so we have no idea what files to add to the watch set till we've built a program once and gotten the full file set. Still, this helps pick up changes that happen any time after the first build, which helps a lot in --watch , if not --incremental.

state.moduleResolutionCache.getPackageJsonInfoCache().entries(),
([path, data]) => ([state.host.realpath ? toPath(state, state.host.realpath(path)) : path, data] as const)
));

if (state.watch) {
state.builderPrograms.set(projectPath, program);
}
Expand Down Expand Up @@ -1192,12 +1207,14 @@ namespace ts {
watchExtendedConfigFiles(state, projectPath, config);
watchWildCardDirectories(state, project, projectPath, config);
watchInputFiles(state, project, projectPath, config);
watchPackageJsonFiles(state, project, projectPath, config);
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 incorrect location for these? Because program is yet not created so this won't be correct list? And all the package.json watches will be deleted.

Copy link
Member Author

Choose a reason for hiding this comment

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

How so? This is the only place we add tsbuild watchers, afaik. It's a known limitation that we don't have a full list on first build (or, more like, we have an empty list because we don't have a good way to calculate a list).

Copy link
Member

Choose a reason for hiding this comment

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

Because even though project config file was parsed to calculate the new filenames, options etc, the program was not created. that happens later during building of project so this cache is old cache and doing this has no effect?

}
else if (reloadLevel === ConfigFileProgramReloadLevel.Partial) {
// Update file names
config.fileNames = getFileNamesFromConfigSpecs(config.options.configFile!.configFileSpecs!, getDirectoryPath(project), config.options, state.parseConfigFileHost);
updateErrorForNoInputFiles(config.fileNames, project, config.options.configFile!.configFileSpecs!, config.errors, canJsonReportNoInputFiles(config.raw));
watchInputFiles(state, project, projectPath, config);
watchPackageJsonFiles(state, project, projectPath, config);
}

const status = getUpToDateStatus(state, config, projectPath);
Expand Down Expand Up @@ -1490,6 +1507,13 @@ namespace ts {
// Check extended config time
const extendedConfigStatus = forEach(project.options.configFile!.extendedSourceFiles || emptyArray, configFile => checkConfigFileUpToDateStatus(state, configFile, oldestOutputFileTime, oldestOutputFileName));
if (extendedConfigStatus) return extendedConfigStatus;

// Check package file time
const dependentPackageFileStatus = forEach(
state.lastCachedPackageJsonLookups.get(resolvedPath) || emptyArray,
([path]) => checkConfigFileUpToDateStatus(state, path, oldestOutputFileTime, oldestOutputFileName)
);
if (dependentPackageFileStatus) return dependentPackageFileStatus;
}

if (!force && !state.buildInfoChecked.has(resolvedPath)) {
Expand Down Expand Up @@ -1862,6 +1886,25 @@ namespace ts {
);
}

function watchPackageJsonFiles(state: SolutionBuilderState, resolved: ResolvedConfigFileName, resolvedPath: ResolvedConfigFilePath, parsed: ParsedCommandLine) {
if (!state.watch || !state.lastCachedPackageJsonLookups) return;
mutateMap(
getOrCreateValueMapFromConfigFileMap(state.allWatchedPackageJsonFiles, resolvedPath),
new Map(state.lastCachedPackageJsonLookups.get(resolvedPath)),
{
createNewValue: (path, _input) => state.watchFile(
path,
() => invalidateProjectAndScheduleBuilds(state, resolvedPath, ConfigFileProgramReloadLevel.Full),
PollingInterval.High,
parsed?.watchOptions,
WatchType.PackageJson,
resolved
),
onDeleteValue: closeFileWatcher,
}
);
}

function startWatching(state: SolutionBuilderState, buildOrder: AnyBuildOrder) {
if (!state.watchAllProjectsPending) return;
state.watchAllProjectsPending = false;
Expand All @@ -1877,6 +1920,9 @@ namespace ts {

// Watch input files
watchInputFiles(state, resolved, resolvedPath, cfg);

// Watch package json files
watchPackageJsonFiles(state, resolved, resolvedPath, cfg);
}
}
}
Expand All @@ -1886,6 +1932,7 @@ namespace ts {
clearMap(state.allWatchedExtendedConfigFiles, closeFileWatcherOf);
clearMap(state.allWatchedWildcardDirectories, watchedWildcardDirectories => clearMap(watchedWildcardDirectories, closeFileWatcherOf));
clearMap(state.allWatchedInputFiles, watchedWildcardDirectories => clearMap(watchedWildcardDirectories, closeFileWatcher));
clearMap(state.allWatchedPackageJsonFiles, watchedPacageJsonFiles => clearMap(watchedPacageJsonFiles, closeFileWatcher));
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3895,6 +3895,8 @@ namespace ts {
/* @internal */
getMissingFilePaths(): readonly Path[];
/* @internal */
getModuleResolutionCache(): ModuleResolutionCache | undefined;
/* @internal */
getFilesByNameMap(): ESMap<string, SourceFile | false | undefined>;

/**
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ namespace ts {
ConfigFileOfReferencedProject: "Config file of referened project",
ExtendedConfigOfReferencedProject: "Extended config file of referenced project",
WildcardDirectoryOfReferencedProject: "Wild card directory of referenced project",
PackageJson: "package.json file",
};

export interface WatchTypeRegistry {
Expand All @@ -431,6 +432,7 @@ namespace ts {
ConfigFileOfReferencedProject: "Config file of referened project",
ExtendedConfigOfReferencedProject: "Extended config file of referenced project",
WildcardDirectoryOfReferencedProject: "Wild card directory of referenced project",
PackageJson: "package.json file",
}

interface WatchFactory<X, Y = undefined> extends ts.WatchFactory<X, Y> {
Expand Down
29 changes: 27 additions & 2 deletions src/compiler/watchPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,14 @@ namespace ts {
let builderProgram: T;
let reloadLevel: ConfigFileProgramReloadLevel; // level to indicate if the program needs to be reloaded from config file/just filenames etc
let missingFilesMap: ESMap<Path, FileWatcher>; // Map of file watchers for the missing files
let packageJsonMap: ESMap<Path, FileWatcher>; // map of watchers for package json files used in module resolution
let watchedWildcardDirectories: ESMap<string, WildcardDirectoryWatcher>; // map of watchers for the wild card directories in the config file
let timerToUpdateProgram: any; // timer callback to recompile the program
let timerToInvalidateFailedLookupResolutions: any; // timer callback to invalidate resolutions for changes in failed lookup locations
let parsedConfigs: ESMap<Path, ParsedConfig> | undefined; // Parsed commandline and watching cached for referenced projects
let sharedExtendedConfigFileWatchers: ESMap<Path, SharedExtendedConfigFileWatcher<Path>>; // Map of file watchers for extended files, shared between different referenced projects
let extendedConfigCache = host.extendedConfigCache; // Cache for extended config evaluation
let changesAffectResolution = false; // Flag for indicating non-config changes affect module resolution

const sourceFilesCache = new Map<string, HostFileInfo>(); // Cache that stores the source file and version info
let missingFilePathsRequestedForRelease: Path[] | undefined; // These paths are held temporarily so that we can remove the entry from source file cache if the file is not tracked by missing files
Expand Down Expand Up @@ -419,13 +421,13 @@ namespace ts {
const program = getCurrentBuilderProgram();
if (hasChangedCompilerOptions) {
newLine = updateNewLine();
if (program && changesAffectModuleResolution(program.getCompilerOptions(), compilerOptions)) {
if (program && (changesAffectResolution || changesAffectModuleResolution(program.getCompilerOptions(), compilerOptions))) {
resolutionCache.clear();
}
}

// All resolutions are invalid if user provided resolutions
const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution(userProvidedResolution);
const hasInvalidatedResolution = resolutionCache.createHasInvalidatedResolution(userProvidedResolution || changesAffectResolution);
if (isProgramUptoDate(getCurrentProgram(), rootFileNames, compilerOptions, getSourceVersion, fileExists, hasInvalidatedResolution, hasChangedAutomaticTypeDirectiveNames, getParsedCommandLine, projectReferences)) {
if (hasChangedConfigFileParsingErrors) {
builderProgram = createProgram(/*rootNames*/ undefined, /*options*/ undefined, compilerHost, builderProgram, configFileParsingDiagnostics, projectReferences);
Expand All @@ -436,6 +438,8 @@ namespace ts {
createNewProgram(hasInvalidatedResolution);
}

changesAffectResolution = false; // reset for next sync

if (host.afterProgramCreate && program !== builderProgram) {
host.afterProgramCreate(builderProgram);
}
Expand All @@ -457,10 +461,13 @@ namespace ts {
compilerHost.hasInvalidatedResolution = hasInvalidatedResolution;
compilerHost.hasChangedAutomaticTypeDirectiveNames = hasChangedAutomaticTypeDirectiveNames;
builderProgram = createProgram(rootFileNames, compilerOptions, compilerHost, builderProgram, configFileParsingDiagnostics, projectReferences);
// map package json cache entries to their realpaths so we don't try to watch across symlinks
const packageCacheEntries = map(resolutionCache.getModuleResolutionCache().getPackageJsonInfoCache().entries(), ([path, data]) => ([compilerHost.realpath ? toPath(compilerHost.realpath(path)) : path, data] as const));
resolutionCache.finishCachingPerDirectoryResolution();

// Update watches
updateMissingFilePathsWatch(builderProgram.getProgram(), missingFilesMap || (missingFilesMap = new Map()), watchMissingFilePath);
updatePackageJsonWatch(packageCacheEntries, packageJsonMap || (packageJsonMap = new Map()), watchPackageJsonLookupPath);
if (needsUpdateInTypeRootWatch) {
resolutionCache.updateTypeRootsWatch();
}
Expand Down Expand Up @@ -823,6 +830,24 @@ namespace ts {
watchFilePath(missingFilePath, missingFilePath, onMissingFileChange, PollingInterval.Medium, watchOptions, WatchType.MissingFile);
}

function watchPackageJsonLookupPath(packageJsonPath: Path) {
// If the package.json is pulled into the compilation itself (eg, via json imports), don't add a second watcher here
return sourceFilesCache.has(packageJsonPath) ?
noopFileWatcher :
watchFilePath(packageJsonPath, packageJsonPath, onPackageJsonChange, PollingInterval.High, watchOptions, WatchType.PackageJson);
}

function onPackageJsonChange(fileName: string, eventKind: FileWatcherEventKind, path: Path) {
updateCachedSystemWithFile(fileName, path, eventKind);

// package.json changes invalidate module resolution and can change the set of loaded files
// so if we witness a change to one, we have to do a full reload
reloadLevel = ConfigFileProgramReloadLevel.Full;
Copy link
Member

Choose a reason for hiding this comment

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

This seems incorrect.. Full Reload means read config file again to calculate file names.. Why do we need to do that for package.json changes.
Also changesAffectResolution seems very unoptimized way of saying resolve all modules again. Need smarter way to invalidate resolutions.. Esp when someone is not using the node12+ stuff.. looks like every npm install will be problematic? (it many times spans across multiple updates if its many packages)

Copy link
Member Author

@weswigham weswigham Mar 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

A package.json change can change what arbitrary module specifiers in the program resolve to (when main, exports, or typesVersions change) and which imports are valid in arbitrary files (when type changes), so, yeah, a package.json change can invalidate the whole program.

Copy link
Member Author

Choose a reason for hiding this comment

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

And do note, only the exports and type fields are new - we failed to pickup main and typesVersions changes and invalidations from package.json files entirely before. A fix for that actually landed before any of the node12 stuff.

Copy link
Member

Choose a reason for hiding this comment

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

But invalidating whole program is different from reading config file and getting initial set which is what ConfigFileProgramReloadLevel.Full means

Copy link
Member

Choose a reason for hiding this comment

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

Also package.json main and typeversion use to be rare and we use to watch failed lookups and in most cases that would cover it.. otherwise the npm install would need to re-do module resolutions for whole program.. Wouldnt it be still same that package.json affects certain files for module resolutions and not all.. eg. changing node's package.json shouldnt need to do module resolution in file that doesn't use node stuff..

Copy link
Member Author

Choose a reason for hiding this comment

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

package.json main is about as far from "rare" as you can get - every package has one. :P

You would be right that we probably don't need to watch them if moduleResolution isn't node/node12/nodenext though. Not that many people use TS without one of those set, since module: commonjs (and thus moduleResolution: node) is the norm. Except I believe @types resolution unconditionally uses node-style resolution anyway; so I think we actually need to watch the package.json documents in those folders unconditionally...

changesAffectResolution = true;
// Update the program
scheduleProgramUpdate();
}

function onMissingFileChange(fileName: string, eventKind: FileWatcherEventKind, missingFilePath: Path) {
updateCachedSystemWithFile(fileName, missingFilePath, eventKind);

Expand Down
19 changes: 19 additions & 0 deletions src/compiler/watchUtilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,25 @@ namespace ts {
});
}

/**
* Updates watchers based on the package json files used in module resolution
*/
export function updatePackageJsonWatch(
lookups: readonly (readonly [Path, object | boolean])[],
packageJsonWatches: ESMap<Path, FileWatcher>,
createPackageJsonWatch: (packageJsonPath: Path, data: object | boolean) => FileWatcher,
) {
const newMap = new Map(lookups);
mutateMap(
packageJsonWatches,
newMap,
{
createNewValue: createPackageJsonWatch,
onDeleteValue: closeFileWatcher
}
);
}

/**
* Updates the existing missing file watches with the new set of missing files after new program is created
*/
Expand Down
Loading