Skip to content

Pipeable #8976

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.

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

Merged
merged 1 commit into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"@types/node": "^6.0.84",
"@types/request": "~2.0.0",
"@types/semver": "^5.3.30",
"@types/source-map": "^0.5.0",
"@types/source-map": "0.5.2",
"@types/webpack": "^3.0.5",
"@types/webpack-sources": "^0.1.3",
"conventional-changelog": "1.1.0",
Expand Down
2 changes: 0 additions & 2 deletions packages/@angular/cli/commands/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const stringUtils = require('ember-cli-string-utils');
import { oneLine } from 'common-tags';
import { CliConfig } from '../models/config';

import 'rxjs/add/observable/of';
import 'rxjs/add/operator/ignoreElements';
import {
getCollection,
getEngineHost
Expand Down
2 changes: 1 addition & 1 deletion packages/@angular/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"postcss-url": "^7.1.2",
"raw-loader": "^0.5.1",
"resolve": "^1.1.7",
"rxjs": "^5.5.2",
"rxjs": "^5.5.6",
"sass-loader": "^6.0.6",
"semver": "^5.1.0",
"silent-error": "^1.0.0",
Expand Down
6 changes: 3 additions & 3 deletions packages/@angular/cli/tasks/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ export interface CliLintConfig {
export class LintTaskOptions {
fix: boolean;
force: boolean;
format? = 'prose';
silent? = false;
typeCheck? = false;
format ? = 'prose';
silent ? = false;
typeCheck ? = false;
configs: Array<CliLintConfig>;
}

Expand Down
29 changes: 16 additions & 13 deletions packages/@angular/cli/tasks/schematic-run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ import {
Tree
} from '@angular-devkit/schematics';
import { FileSystemHost } from '@angular-devkit/schematics/tools';
import { Observable } from 'rxjs/Observable';
import { of as observableOf } from 'rxjs/observable/of';
import * as path from 'path';
import chalk from 'chalk';
import { CliConfig } from '../models/config';
import 'rxjs/add/operator/concatMap';
import 'rxjs/add/operator/map';
import { concat, concatMap, ignoreElements, map } from 'rxjs/operators';
import { getCollection, getSchematic } from '../utilities/schematics';

const { green, red, yellow } = chalk;
Expand Down Expand Up @@ -58,7 +57,7 @@ export default Task.extend({
const opts = { ...taskOptions, ...preppedOptions };

const tree = emptyHost ? new EmptyTree() : new FileSystemTree(new FileSystemHost(workingDir));
const host = Observable.of(tree);
const host = observableOf(tree);

const dryRunSink = new DryRunSink(workingDir, opts.force);
const fsSink = new FileSystemSink(workingDir, opts.force);
Expand Down Expand Up @@ -111,22 +110,26 @@ export default Task.extend({
});

return new Promise((resolve, reject) => {
schematic.call(opts, host)
.map((tree: Tree) => Tree.optimize(tree))
.concatMap((tree: Tree) => {
return dryRunSink.commit(tree).ignoreElements().concat(Observable.of(tree));
})
.concatMap((tree: Tree) => {
schematic.call(opts, host).pipe(
map((tree: Tree) => Tree.optimize(tree)),
concatMap((tree: Tree) => {
return dryRunSink.commit(tree).pipe(
ignoreElements(),
concat(observableOf(tree)));
}),
concatMap((tree: Tree) => {
if (!error) {
// Output the logging queue.
loggingQueue.forEach(log => ui.writeLine(` ${log.color(log.keyword)} ${log.message}`));
}

if (opts.dryRun || error) {
return Observable.of(tree);
return observableOf(tree);
}
return fsSink.commit(tree).ignoreElements().concat(Observable.of(tree));
})
return fsSink.commit(tree).pipe(
ignoreElements(),
concat(observableOf(tree)));
}))
.subscribe({
error(err) {
ui.writeLine(red(`Error: ${err.message}`));
Expand Down
2 changes: 0 additions & 2 deletions packages/@angular/cli/utilities/schematics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import {
validateOptionsWithSchema
} from '@angular-devkit/schematics/tools';
import { SchemaClassFactory } from '@ngtools/json-schema';
import 'rxjs/add/operator/concatMap';
import 'rxjs/add/operator/map';

const SilentError = require('silent-error');

Expand Down
10 changes: 4 additions & 6 deletions packages/@ngtools/logger/src/console-logger-stack.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {LogEntry, Logger} from './logger';
import {ConsoleLoggerStack} from './console-logger-stack';
import {NullLogger} from './null-logger';
import {toArray} from 'rxjs/operators';
Copy link
Member

Choose a reason for hiding this comment

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

This should be an import directly from 'rxjs/operators/toArray'

Choose a reason for hiding this comment

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

no, importing from 'rxjs/operators' is fine

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 to maintain parity with angular/material2, angular/flex-layout, and angular/preboot, starting with angular/components#8160. It seems like it's an open question though on that thread, and will probably become irrelevant as RxJS 6 comes closer

Choose a reason for hiding this comment

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

Importing from operators does cause a larger bundle currently but from memory that's a bug at the moment and should be fixed. With that in mind I think the more ergonomic approach should be the desired kne

Copy link
Member

Choose a reason for hiding this comment

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

Please note also that this is a node application not a web application. Bundle size is irrelevant.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's more about setting a precedent for best practices, as a lot of developers look to first-party packages like this for guidance (node app or no), especially when it comes to RxJS. Again, for this at least it seems to be a non-starter, the only reason I'm leaving this comment up is to archive the discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Best practices for RxJS 6 is to import {toArray} from 'rxjs'. I'm personally okay with this as it's likely the whole Angular ecosystem will move towards that over the next quarter or two.



describe('ConsoleLoggerStack', () => {
it('works', (done: DoneFn) => {
const logger = ConsoleLoggerStack.start('test');
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([
Expand All @@ -26,8 +26,7 @@ describe('ConsoleLoggerStack', () => {
const oldConsoleLog = console.log;
const logger = ConsoleLoggerStack.start('test');
expect(console.log).not.toBe(oldConsoleLog);
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([
Expand Down Expand Up @@ -55,8 +54,7 @@ describe('ConsoleLoggerStack', () => {
const logger = new Logger('test');
ConsoleLoggerStack.start(logger);
expect(console.log).not.toBe(oldConsoleLog);
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([
Expand Down
4 changes: 2 additions & 2 deletions packages/@ngtools/logger/src/indent.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {LogEntry, Logger} from './logger';
import {IndentLogger} from './indent';
import {toArray} from 'rxjs/operators';


describe('IndentSpec', () => {
it('works', (done: DoneFn) => {
const logger = new IndentLogger('test');
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([
Expand Down
19 changes: 9 additions & 10 deletions packages/@ngtools/logger/src/indent.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { map } from 'rxjs/operators';
import {Logger} from './logger';

import 'rxjs/add/operator/map';


/**
* Keep an map of indentation => array of indentations based on the level.
Expand All @@ -18,20 +17,20 @@ export class IndentLogger extends Logger {
super(name, parent);

indentationMap[indentation] = indentationMap[indentation] || [''];
const map = indentationMap[indentation];
const indentMap = indentationMap[indentation];

this._observable = this._observable.map(entry => {
this._observable = this._observable.pipe(map(entry => {
const l = entry.path.length;
if (l >= map.length) {
let current = map[map.length - 1];
while (l >= map.length) {
if (l >= indentMap.length) {
let current = indentMap[indentMap.length - 1];
while (l >= indentMap.length) {
current += indentation;
map.push(current);
indentMap.push(current);
}
}

entry.message = map[l] + entry.message;
entry.message = indentMap[l] + entry.message;
return entry;
});
}));
}
}
9 changes: 3 additions & 6 deletions packages/@ngtools/logger/src/logger.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import {Logger, JsonValue} from './logger';
import 'rxjs/add/operator/toArray';
import 'rxjs/add/operator/toPromise';
import {toArray} from 'rxjs/operators';


describe('Logger', () => {
it('works', (done: DoneFn) => {
const logger = new Logger('test');
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: JsonValue[]) => {
expect(observed).toEqual([
Expand All @@ -25,8 +23,7 @@ describe('Logger', () => {
it('works with children', (done: DoneFn) => {
const logger = new Logger('test');
let hasCompleted = false;
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: JsonValue[]) => {
expect(observed).toEqual([
Expand Down
7 changes: 3 additions & 4 deletions packages/@ngtools/logger/src/null-logger.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {NullLogger} from './null-logger';
import {LogEntry, Logger} from './logger';
import {toArray} from 'rxjs/operators';


describe('NullLogger', () => {
it('works', (done: DoneFn) => {
const logger = new NullLogger();
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([]);
Expand All @@ -20,8 +20,7 @@ describe('NullLogger', () => {

it('nullifies children', (done: DoneFn) => {
const logger = new Logger('test');
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([]);
Expand Down
9 changes: 3 additions & 6 deletions packages/@ngtools/logger/src/null-logger.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import {Logger} from './logger';

import {Observable} from 'rxjs/Observable';

import 'rxjs/add/observable/empty';
import { empty } from 'rxjs/observable/empty';
import { Logger } from './logger';


export class NullLogger extends Logger {
constructor(parent: Logger | null = null) {
super('', parent);
this._observable = Observable.empty();
this._observable = empty();
}
}
15 changes: 6 additions & 9 deletions packages/@ngtools/logger/src/transform-logger.spec.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import {TransformLogger} from './transform-logger';
import {LogEntry} from './logger';

import 'rxjs/add/operator/filter';
import 'rxjs/add/operator/map';
import {filter, map, toArray} from 'rxjs/operators';


describe('TransformLogger', () => {
it('works', (done: DoneFn) => {
const logger = new TransformLogger('test', stream => {
return stream
.filter(entry => entry.message != 'hello')
.map(entry => {
return stream.pipe(
filter(entry => entry.message != 'hello'),
map(entry => {
entry.message += '1';
return entry;
});
}));
});
logger
.toArray()
logger.pipe(toArray())
.toPromise()
.then((observed: LogEntry[]) => {
expect(observed).toEqual([
Expand Down
2 changes: 1 addition & 1 deletion packages/@ngtools/webpack/src/angular_compiler_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ export class AngularCompilerPlugin implements Tapable {
this._entryModule = this._options.entryModule;
} else if (this._compilerOptions.entryModule) {
this._entryModule = path.resolve(this._basePath,
this._compilerOptions.entryModule);
this._compilerOptions.entryModule as string); // temporary cast for type issue
}

// Set platform.
Expand Down
4 changes: 2 additions & 2 deletions scripts/test-commit-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const validateCommitMessage = require('./validate-commit-message');
const execSync = require('child_process').execSync;
const chalk = require('chalk');
const Logger = require('@ngtools/logger').Logger;
require('rxjs/add/operator/filter');
const filter = require('rxjs/operators').filter;

// Configure logger
const logger = new Logger('test-commit-messages');
Expand All @@ -25,7 +25,7 @@ logger.subscribe((entry) => {
});

logger
.filter((entry) => entry.level === 'fatal')
.pipe(filter((entry) => entry.level === 'fatal'))
.subscribe(() => {
process.stderr.write('A fatal error happened. See details above.');
process.exit(1);
Expand Down
4 changes: 2 additions & 2 deletions scripts/test-licenses.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path');
const chalk = require('chalk');
const spdxSatisfies = require('spdx-satisfies');
const Logger = require('@ngtools/logger').Logger;
require('rxjs/add/operator/filter');
const filter = require('rxjs/operators').filter;

// Configure logger
const logger = new Logger('test-licenses');
Expand All @@ -23,7 +23,7 @@ logger.subscribe((entry) => {
});

logger
.filter((entry) => entry.level == 'fatal')
.pipe(filter((entry) => entry.level == 'fatal'))
.subscribe(() => {
process.stderr.write('A fatal error happened. See details above.');
process.exit(1);
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/tests/basic/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ export default function () {
'src/app/app.component.ts': `
import { Component } from '@angular/core';
import { Http, Response } from '@angular/http';
import 'rxjs/add/operator/map';

@Component({
selector: 'app-root',
Expand Down
5 changes: 2 additions & 3 deletions tests/e2e_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import * as path from 'path';
import {setGlobalVariable} from './e2e/utils/env';

// RxJS
import 'rxjs/add/operator/filter';
import 'rxjs/add/observable/empty';
import {filter} from 'rxjs/operators';


Error.stackTraceLimit = Infinity;
Expand Down Expand Up @@ -73,7 +72,7 @@ process.exitCode = 255;


ConsoleLoggerStack.start(new IndentLogger('name'))
.filter((entry: LogEntry) => (entry.level != 'debug' || argv.verbose))
.pipe(filter((entry: LogEntry) => (entry.level != 'debug' || argv.verbose)))
.subscribe((entry: LogEntry) => {
let color: (s: string) => string = white;
let output = process.stdout;
Expand Down
Loading