From 4de8dc3554f62d4c6b3abfeb458c7e77cffce4cc Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 3 Sep 2020 20:15:31 +0100 Subject: [PATCH] fix(localize): do not expose NodeJS typings in $localize runtime code (#38700) A recent change to `@angular/localize` brought in the `AbsoluteFsPath` type from the `@angular/compiler-cli`. But this brought along with it a reference to NodeJS typings - specifically the `FileSystem` interface refers to the `Buffer` type from NodeJS. This affects compilation of `@angular/localize` code that will be run in the browser - for example projects that reference `loadTranslations()`. The compilation breaks if the NodeJS typings are not included in the build. Clearly it is not desirable to have these typings included when the project is not targeting NodeJS. This commit replaces references to the NodeJS `Buffer` type with `Uint8Array`, which is available across all platforms and is actually the super-class of `Buffer`. Fixes #38692 PR Close #38700 --- .../src/ngtsc/file_system/src/invalid_file_system.ts | 4 ++-- .../src/ngtsc/file_system/src/node_js_file_system.ts | 4 ++-- packages/compiler-cli/src/ngtsc/file_system/src/types.ts | 4 ++-- .../src/ngtsc/file_system/testing/src/mock_file_system.ts | 8 ++++---- .../translate/asset_files/asset_translation_handler.ts | 8 ++++---- .../source_files/source_file_translation_handler.ts | 8 ++++---- packages/localize/src/tools/src/translate/translator.ts | 6 +++--- .../localize/src/tools/test/translate/translator_spec.ts | 6 +++--- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts index b55223efa8cad..5dadaa1fbdb4b 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts @@ -22,10 +22,10 @@ export class InvalidFileSystem implements FileSystem { readFile(path: AbsoluteFsPath): string { throw makeError(); } - readFileBuffer(path: AbsoluteFsPath): Buffer { + readFileBuffer(path: AbsoluteFsPath): Uint8Array { throw makeError(); } - writeFile(path: AbsoluteFsPath, data: string|Buffer, exclusive?: boolean): void { + writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive?: boolean): void { throw makeError(); } removeFile(path: AbsoluteFsPath): void { diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts index 44ad46a16dd5f..444092831978d 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts @@ -23,10 +23,10 @@ export class NodeJSFileSystem implements FileSystem { readFile(path: AbsoluteFsPath): string { return fs.readFileSync(path, 'utf8'); } - readFileBuffer(path: AbsoluteFsPath): Buffer { + readFileBuffer(path: AbsoluteFsPath): Uint8Array { return fs.readFileSync(path); } - writeFile(path: AbsoluteFsPath, data: string|Buffer, exclusive: boolean = false): void { + writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive: boolean = false): void { fs.writeFileSync(path, data, exclusive ? {flag: 'wx'} : undefined); } removeFile(path: AbsoluteFsPath): void { diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/types.ts b/packages/compiler-cli/src/ngtsc/file_system/src/types.ts index b232db828c832..bdc660e3011f4 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/types.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/types.ts @@ -37,8 +37,8 @@ export type PathSegment = BrandedPath<'PathSegment'>; export interface FileSystem { exists(path: AbsoluteFsPath): boolean; readFile(path: AbsoluteFsPath): string; - readFileBuffer(path: AbsoluteFsPath): Buffer; - writeFile(path: AbsoluteFsPath, data: string|Buffer, exclusive?: boolean): void; + readFileBuffer(path: AbsoluteFsPath): Uint8Array; + writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive?: boolean): void; removeFile(path: AbsoluteFsPath): void; symlink(target: AbsoluteFsPath, path: AbsoluteFsPath): void; readdir(path: AbsoluteFsPath): PathSegment[]; diff --git a/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts index 5d4cc337bac56..9122f531eb8e8 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts @@ -38,16 +38,16 @@ export abstract class MockFileSystem implements FileSystem { } } - readFileBuffer(path: AbsoluteFsPath): Buffer { + readFileBuffer(path: AbsoluteFsPath): Uint8Array { const {entity} = this.findFromPath(path); if (isFile(entity)) { - return Buffer.isBuffer(entity) ? entity : new Buffer(entity); + return entity instanceof Uint8Array ? entity : new Buffer(entity); } else { throw new MockFileSystemError('ENOENT', path, `File "${path}" does not exist.`); } } - writeFile(path: AbsoluteFsPath, data: string|Buffer, exclusive: boolean = false): void { + writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive: boolean = false): void { const [folderPath, basename] = this.splitIntoFolderAndFile(path); const {entity} = this.findFromPath(folderPath); if (entity === null || !isFolder(entity)) { @@ -295,7 +295,7 @@ export type Entity = Folder|File|SymLink; export interface Folder { [pathSegments: string]: Entity; } -export type File = string|Buffer; +export type File = string|Uint8Array; export class SymLink { constructor(public path: AbsoluteFsPath) {} } diff --git a/packages/localize/src/tools/src/translate/asset_files/asset_translation_handler.ts b/packages/localize/src/tools/src/translate/asset_files/asset_translation_handler.ts index 97ae7ba8a6e2c..3b0ddeec70a08 100644 --- a/packages/localize/src/tools/src/translate/asset_files/asset_translation_handler.ts +++ b/packages/localize/src/tools/src/translate/asset_files/asset_translation_handler.ts @@ -17,14 +17,14 @@ import {TranslationBundle, TranslationHandler} from '../translator'; export class AssetTranslationHandler implements TranslationHandler { constructor(private fs: FileSystem) {} - canTranslate(_relativeFilePath: PathSegment|AbsoluteFsPath, _contents: Buffer): boolean { + canTranslate(_relativeFilePath: PathSegment|AbsoluteFsPath, _contents: Uint8Array): boolean { return true; } translate( diagnostics: Diagnostics, _sourceRoot: AbsoluteFsPath, - relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer, outputPathFn: OutputPathFn, - translations: TranslationBundle[], sourceLocale?: string): void { + relativeFilePath: PathSegment|AbsoluteFsPath, contents: Uint8Array, + outputPathFn: OutputPathFn, translations: TranslationBundle[], sourceLocale?: string): void { for (const translation of translations) { this.writeAssetFile( diagnostics, outputPathFn, translation.locale, relativeFilePath, contents); @@ -36,7 +36,7 @@ export class AssetTranslationHandler implements TranslationHandler { private writeAssetFile( diagnostics: Diagnostics, outputPathFn: OutputPathFn, locale: string, - relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer): void { + relativeFilePath: PathSegment|AbsoluteFsPath, contents: Uint8Array): void { try { const outputPath = absoluteFrom(outputPathFn(locale, relativeFilePath)); this.fs.ensureDir(this.fs.dirname(outputPath)); diff --git a/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts b/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts index 4d1e6cc763fb3..18f41636b4dcc 100644 --- a/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts +++ b/packages/localize/src/tools/src/translate/source_files/source_file_translation_handler.ts @@ -27,15 +27,15 @@ export class SourceFileTranslationHandler implements TranslationHandler { TranslatePluginOptions = {...this.translationOptions, missingTranslation: 'ignore'}; constructor(private fs: FileSystem, private translationOptions: TranslatePluginOptions = {}) {} - canTranslate(relativeFilePath: PathSegment|AbsoluteFsPath, _contents: Buffer): boolean { + canTranslate(relativeFilePath: PathSegment|AbsoluteFsPath, _contents: Uint8Array): boolean { return this.fs.extname(relativeFilePath) === '.js'; } translate( diagnostics: Diagnostics, sourceRoot: AbsoluteFsPath, relativeFilePath: PathSegment, - contents: Buffer, outputPathFn: OutputPathFn, translations: TranslationBundle[], + contents: Uint8Array, outputPathFn: OutputPathFn, translations: TranslationBundle[], sourceLocale?: string): void { - const sourceCode = contents.toString('utf8'); + const sourceCode = Buffer.from(contents).toString('utf8'); // A short-circuit check to avoid parsing the file into an AST if it does not contain any // `$localize` identifiers. if (!sourceCode.includes('$localize')) { @@ -97,7 +97,7 @@ export class SourceFileTranslationHandler implements TranslationHandler { private writeSourceFile( diagnostics: Diagnostics, outputPathFn: OutputPathFn, locale: string, - relativeFilePath: PathSegment, contents: string|Buffer): void { + relativeFilePath: PathSegment, contents: string|Uint8Array): void { try { const outputPath = absoluteFrom(outputPathFn(locale, relativeFilePath)); this.fs.ensureDir(this.fs.dirname(outputPath)); diff --git a/packages/localize/src/tools/src/translate/translator.ts b/packages/localize/src/tools/src/translate/translator.ts index a72e1d87bbc5c..6c1d1eb3d6107 100644 --- a/packages/localize/src/tools/src/translate/translator.ts +++ b/packages/localize/src/tools/src/translate/translator.ts @@ -35,7 +35,7 @@ export interface TranslationHandler { * @param relativeFilePath A relative path from the sourceRoot to the resource file to handle. * @param contents The contents of the file to handle. */ - canTranslate(relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer): boolean; + canTranslate(relativeFilePath: PathSegment|AbsoluteFsPath, contents: Uint8Array): boolean; /** * Translate the file at `relativeFilePath` containing `contents`, using the given `translations`, @@ -54,8 +54,8 @@ export interface TranslationHandler { */ translate( diagnostics: Diagnostics, sourceRoot: AbsoluteFsPath, - relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer, outputPathFn: OutputPathFn, - translations: TranslationBundle[], sourceLocale?: string): void; + relativeFilePath: PathSegment|AbsoluteFsPath, contents: Uint8Array, + outputPathFn: OutputPathFn, translations: TranslationBundle[], sourceLocale?: string): void; } /** diff --git a/packages/localize/src/tools/test/translate/translator_spec.ts b/packages/localize/src/tools/test/translate/translator_spec.ts index 0d2353a1658d6..ea42c7e515480 100644 --- a/packages/localize/src/tools/test/translate/translator_spec.ts +++ b/packages/localize/src/tools/test/translate/translator_spec.ts @@ -109,13 +109,13 @@ runInEachFileSystem(() => { log: string[] = []; constructor(private _canTranslate: boolean = true) {} - canTranslate(relativePath: string, contents: Buffer) { - this.log.push(`canTranslate(${relativePath}, ${contents.toString('utf8')})`); + canTranslate(relativePath: string, contents: Uint8Array) { + this.log.push(`canTranslate(${relativePath}, ${contents})`); return this._canTranslate; } translate( - _diagnostics: Diagnostics, rootPath: string, relativePath: string, contents: Buffer, + _diagnostics: Diagnostics, rootPath: string, relativePath: string, contents: Uint8Array, _outputPathFn: OutputPathFn, _translations: TranslationBundle[], sourceLocale?: string) { this.log.push( `translate(${rootPath}, ${relativePath}, ${contents}, ...` +