Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Commit

Permalink
fix(): always serve latest file changes (#1521)
Browse files Browse the repository at this point in the history
When running `ionic serve` it often happens that the transpiled files become out of date. After making a change to one of the TypeScript source files, ionic serve runs, but when looking at the built JavaScript files, the new code is not present. I found that the app-scripts build process kept two file caches. One in the app-script context and one in the so called InMemoryCompilerHost. The first was updated when files changed, but the latter wasn't. The InMemoryCompilerHost is in my opinion superfluous, because it got injected a HybridFileSystem, which nicely uses the app-scripts context fileCache. My solution is to rename the InMemoryCompilerHost to FileSystemCompilerHost and let the injected HybridFileSystem handle the caching. This solution not only fixes the out-of-date-files issue, but also redecuses memory usage of app-scripts.
  • Loading branch information
intodevelopment authored and liamdebeasi committed May 24, 2019
1 parent 5d7ca38 commit 266a871
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 19 deletions.
8 changes: 4 additions & 4 deletions src/aot/aot-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import {

import { HybridFileSystem } from '../util/hybrid-file-system';
import { getInstance as getHybridFileSystem } from '../util/hybrid-file-system-factory';
import { getInMemoryCompilerHostInstance } from './compiler-host-factory';
import { InMemoryCompilerHost } from './compiler-host';
import { getFileSystemCompilerHostInstance } from './compiler-host-factory';
import { FileSystemCompilerHost } from './compiler-host';
import { getFallbackMainContent, replaceBootstrapImpl } from './utils';
import { Logger } from '../logger/logger';
import { printDiagnostics, clearDiagnostics, DiagnosticsType } from '../logger/logger-diagnostics';
Expand All @@ -40,7 +40,7 @@ export async function runAot(context: BuildContext, options: AotOptions) {
const aggregateCompilerOption = Object.assign(tsConfig.options, angularCompilerOptions);

const fileSystem = getHybridFileSystem(false);
const compilerHost = getInMemoryCompilerHostInstance(tsConfig.options);
const compilerHost = getFileSystemCompilerHostInstance(tsConfig.options);
// todo, consider refactoring at some point
const tsProgram = createProgram(tsConfig.fileNames, tsConfig.options, compilerHost);

Expand Down Expand Up @@ -77,7 +77,7 @@ export async function runAot(context: BuildContext, options: AotOptions) {
}
}

function errorCheckProgram(context: BuildContext, tsConfig: TsConfig, compilerHost: InMemoryCompilerHost, cachedProgram: Program) {
function errorCheckProgram(context: BuildContext, tsConfig: TsConfig, compilerHost: FileSystemCompilerHost, cachedProgram: Program) {
// Create a new Program, based on the old one. This will trigger a resolution of all
// transitive modules, which include files that might just have been generated.
const program = createProgram(tsConfig.fileNames, tsConfig.options, compilerHost, cachedProgram);
Expand Down
8 changes: 4 additions & 4 deletions src/aot/compiler-host-factory.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { CompilerOptions } from 'typescript';
import { InMemoryCompilerHost } from './compiler-host';
import { FileSystemCompilerHost } from './compiler-host';
import { getInstance as getFileSystemInstance } from '../util/hybrid-file-system-factory';

let instance: InMemoryCompilerHost = null;
let instance: FileSystemCompilerHost = null;

export function getInMemoryCompilerHostInstance(options: CompilerOptions) {
export function getFileSystemCompilerHostInstance(options: CompilerOptions) {
if (!instance) {
instance = new InMemoryCompilerHost(options, getFileSystemInstance(false));
instance = new FileSystemCompilerHost(options, getFileSystemInstance(false));
}
return instance;
}
10 changes: 1 addition & 9 deletions src/aot/compiler-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ export interface OnErrorFn {
(message: string): void;
}

export class InMemoryCompilerHost implements CompilerHost {
private sourceFileMap: Map<string, SourceFile>;
export class FileSystemCompilerHost implements CompilerHost {
private diskCompilerHost: CompilerHost;

constructor(private options: CompilerOptions, private fileSystem: VirtualFileSystem, private setParentNodes = true) {
this.diskCompilerHost = createCompilerHost(this.options, this.setParentNodes);
this.sourceFileMap = new Map<string, SourceFile>();
}

fileExists(filePath: string): boolean {
Expand Down Expand Up @@ -64,19 +62,13 @@ export class InMemoryCompilerHost implements CompilerHost {

getSourceFile(filePath: string, languageVersion: ScriptTarget, onError?: OnErrorFn) {
filePath = normalize(filePath);
const existingSourceFile = this.sourceFileMap.get(filePath);
if (existingSourceFile) {
return existingSourceFile;
}
// we haven't created a source file for this yet, so try to use what's in memory
const fileContentFromMemory = this.fileSystem.getFileContent(filePath);
if (fileContentFromMemory) {
const typescriptSourceFile = getTypescriptSourceFile(filePath, fileContentFromMemory, languageVersion, this.setParentNodes);
this.sourceFileMap.set(filePath, typescriptSourceFile);
return typescriptSourceFile;
}
const diskSourceFile = this.diskCompilerHost.getSourceFile(filePath, languageVersion, onError);
this.sourceFileMap.set(filePath, diskSourceFile);
return diskSourceFile;
}

Expand Down
4 changes: 2 additions & 2 deletions src/transpile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as path from 'path';

import * as ts from 'typescript';

import { getInMemoryCompilerHostInstance } from './aot/compiler-host-factory';
import { getFileSystemCompilerHostInstance } from './aot/compiler-host-factory';
import { buildJsSourceMaps } from './bundle';
import {
getInjectDeepLinkConfigTypescriptTransform,
Expand Down Expand Up @@ -117,7 +117,7 @@ export function transpileWorker(context: BuildContext, workerConfig: TranspileWo
tsConfig.options.declaration = undefined;

// let's start a new tsFiles object to cache all the transpiled files in
const host = getInMemoryCompilerHostInstance(tsConfig.options);
const host = getFileSystemCompilerHostInstance(tsConfig.options);

if (workerConfig.useTransforms && getBooleanPropertyValue(Constants.ENV_PARSE_DEEPLINKS)) {
// beforeArray.push(purgeDeepLinkDecoratorTSTransform());
Expand Down

1 comment on commit 266a871

@zarko-tg
Copy link

Choose a reason for hiding this comment

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

This's been bugging us for year(s)... 👏

Please sign in to comment.