Skip to content

Commit

Permalink
fix(localize): do not expose NodeJS typings in $localize runtime code (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
petebacondarwin authored and atscott committed Sep 8, 2020
1 parent ab4f953 commit 4de8dc3
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 24 deletions.
Expand Up @@ -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 {
Expand Down
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/src/ngtsc/file_system/src/types.ts
Expand Up @@ -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[];
Expand Down
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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) {}
}
Expand Down
Expand Up @@ -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);
Expand All @@ -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));
Expand Down
Expand Up @@ -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')) {
Expand Down Expand Up @@ -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));
Expand Down
6 changes: 3 additions & 3 deletions packages/localize/src/tools/src/translate/translator.ts
Expand Up @@ -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`,
Expand All @@ -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;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/localize/src/tools/test/translate/translator_spec.ts
Expand Up @@ -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}, ...` +
Expand Down

0 comments on commit 4de8dc3

Please sign in to comment.