Skip to content

Commit

Permalink
fix(ngcc): use preserve whitespaces from tsconfig if provided (#36189)
Browse files Browse the repository at this point in the history
Previously ngcc never preserved whitespaces but this is at odds
with how the ViewEngine compiler works. In ViewEngine, library
templates are recompiled with the current application's tsconfig
settings, which meant that whitespace preservation could be set
in the application tsconfig file.

This commit allows ngcc to use the `preserveWhitespaces` setting
from tsconfig when compiling library templates. One should be aware
that this disallows different projects with different tsconfig settings
to share the same node_modules folder, with regard to whitespace
preservation. But this is already the case in the current ngcc since
this configuration is hard coded right now.

Fixes #35871

PR Close #36189
  • Loading branch information
petebacondarwin authored and mhevery committed Mar 24, 2020
1 parent fb70083 commit aef4323
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 6 deletions.
Expand Up @@ -8,6 +8,7 @@
import {ConstantPool} from '@angular/compiler';
import * as ts from 'typescript';

import {ParsedConfiguration} from '../../..';
import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
Expand Down Expand Up @@ -55,6 +56,7 @@ export class DecorationAnalyzer {
private rootDirs = this.bundle.rootDirs;
private packagePath = this.bundle.entryPoint.package;
private isCore = this.bundle.isCore;
private compilerOptions = this.tsConfig !== null? this.tsConfig.options: {};

moduleResolver =
new ModuleResolver(this.program, this.options, this.host, /* moduleResolutionCache */ null);
Expand Down Expand Up @@ -87,7 +89,7 @@ export class DecorationAnalyzer {
new ComponentDecoratorHandler(
this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader,
this.scopeRegistry, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs,
/* defaultPreserveWhitespaces */ false,
!!this.compilerOptions.preserveWhitespaces,
/* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat,
this.moduleResolver, this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER,
NOOP_DEPENDENCY_TRACKER, this.injectableRegistry, /* annotateForClosureCompiler */ false),
Expand Down Expand Up @@ -123,7 +125,8 @@ export class DecorationAnalyzer {
constructor(
private fs: FileSystem, private bundle: EntryPointBundle,
private reflectionHost: NgccReflectionHost, private referencesRegistry: ReferencesRegistry,
private diagnosticHandler: (error: ts.Diagnostic) => void = () => {}) {}
private diagnosticHandler: (error: ts.Diagnostic) => void = () => {},
private tsConfig: ParsedConfiguration|null = null) {}

/**
* Analyze a program to find all the decorated files should be transformed.
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/ngcc/src/main.ts
Expand Up @@ -307,7 +307,7 @@ export function mainNgcc(
const createCompileFn: CreateCompileFn = onTaskCompleted => {
const fileWriter = getFileWriter(
fileSystem, logger, pkgJsonUpdater, createNewEntryPointFormats, errorOnFailedEntryPoint);
const transformer = new Transformer(fileSystem, logger);
const transformer = new Transformer(fileSystem, logger, tsConfig);

return (task: Task) => {
const {entryPoint, formatProperty, formatPropertiesToMarkAsProcessed, processDts} = task;
Expand Down
7 changes: 5 additions & 2 deletions packages/compiler-cli/ngcc/src/packages/transformer.ts
Expand Up @@ -7,6 +7,7 @@
*/
import * as ts from 'typescript';

import {ParsedConfiguration} from '../../..';
import {FileSystem} from '../../../src/ngtsc/file_system';
import {TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {DecorationAnalyzer} from '../analysis/decoration_analyzer';
Expand Down Expand Up @@ -63,7 +64,9 @@ export type TransformResult = {
* - Some formats may contain multiple "modules" in a single file.
*/
export class Transformer {
constructor(private fs: FileSystem, private logger: Logger) {}
constructor(
private fs: FileSystem, private logger: Logger,
private tsConfig: ParsedConfiguration|null = null) {}

/**
* Transform the source (and typings) files of a bundle.
Expand Down Expand Up @@ -146,7 +149,7 @@ export class Transformer {
const diagnostics: ts.Diagnostic[] = [];
const decorationAnalyzer = new DecorationAnalyzer(
this.fs, bundle, reflectionHost, referencesRegistry,
diagnostic => diagnostics.push(diagnostic));
diagnostic => diagnostics.push(diagnostic), this.tsConfig);
const decorationAnalyses = decorationAnalyzer.analyzeProgram();

const moduleWithProvidersAnalyzer =
Expand Down
40 changes: 39 additions & 1 deletion packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -1373,6 +1373,44 @@ runInEachFileSystem(() => {
});
});

describe('whitespace preservation', () => {
it('should default not to preserve whitespace', () => {
mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']});
expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
es2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(fs.readFile(_('/dist/local-package/index.js')))
.toMatch(/ɵɵtext\(\d+, " Hello\\n"\);/);
});

it('should preserve whitespace if set in a loaded tsconfig.json', () => {
fs.writeFile(
_('/tsconfig.json'),
JSON.stringify({angularCompilerOptions: {preserveWhitespaces: true}}));
mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']});
expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
es2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(fs.readFile(_('/dist/local-package/index.js')))
.toMatch(/ɵɵtext\(\d+, "\\n Hello\\n"\);/);
});

it('should not preserve whitespace if set to false in a loaded tsconfig.json', () => {
fs.writeFile(
_('/tsconfig.json'),
JSON.stringify({angularCompilerOptions: {preserveWhitespaces: false}}));
mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']});
expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
es2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(fs.readFile(_('/dist/local-package/index.js')))
.toMatch(/ɵɵtext\(\d+, " Hello\\n"\);/);
});
});

describe('with configuration files', () => {
it('should process a configured deep-import as an entry-point', () => {
loadTestFiles([
Expand Down Expand Up @@ -1883,7 +1921,7 @@ runInEachFileSystem(() => {
{
name: _('/dist/local-package/index.js'),
contents:
`import {Component} from '@angular/core';\nexport class AppComponent {};\nAppComponent.decorators = [\n{ type: Component, args: [{selector: 'app', template: '<h2>Hello</h2>'}] }\n];`
`import {Component} from '@angular/core';\nexport class AppComponent {};\nAppComponent.decorators = [\n{ type: Component, args: [{selector: 'app', template: '<h2>\\n Hello\\n</h2>'}] }\n];`
},
{
name: _('/dist/local-package/index.d.ts'),
Expand Down

0 comments on commit aef4323

Please sign in to comment.