Use AsyncTransformStream rather than stream.Transform #210
Conversation
|
(and yes, the formatting is kinda unfortunate. filed as angular/clang-format#44 ) |
gulpfile.js
Outdated
| 'mz', 'multipipe', 'polymer-bundler', | ||
| ]}).then((result) => { | ||
| let invalidFiles = Object.keys(result.invalidFiles) || []; | ||
| let invalidJsFiles = invalidFiles.filter((f) => f.endsWith('.js')); |
There was a problem hiding this comment.
done, here and elsewhere (updated to new tslint and got it clean)
src/analyzer.ts
Outdated
| @@ -22,7 +22,7 @@ import {parseUrl} from 'polymer-analyzer/lib/utils'; | |||
| import * as logging from 'plylog'; | |||
| import {ProjectConfig} from 'polymer-project-config'; | |||
|
|
|||
| import {VinylReaderTransform} from './streams'; | |||
| import {VinylReaderTransform, AsyncTransformStream} from './streams'; | |||
There was a problem hiding this comment.
Doesn't clang sort these?
There was a problem hiding this comment.
we had a require mixed in with the other imports. moved it down and now it sorts
src/streams.ts
Outdated
| // returns early? | ||
| transformDonePromise.then(() => { | ||
| this._writingFinished.resolve(undefined); | ||
| }, (err) => this.emit('error', err)); |
There was a problem hiding this comment.
Just want to be clear, this error is if there's a problem inside _transformIter. As an implementer of an AsyncTransformStream, if I wanted to handle errors inside _transformIter and keep on trucking, I would just this.emit('error', whatever) myself and continue iterating until all _inputs consumed?
src/streams.ts
Outdated
| } | ||
| })(); | ||
| // TODO(rictic): blindly drain the rest of the inputs if _transformIter | ||
| // returns early? |
There was a problem hiding this comment.
Maybe we provide an explicit method to drain remaining inputs and throw an error/reject the promise if the inputs are not empty (i.e. if its closed?) as a guard against accidental early exits? Also, why don't expose closed state on the AsyncQueue even as a readonly public getter?
if (!this._inputs.closed) {
this._writingFinished.reject(new Error('Unfinished inputs queue.'));
} else {
this._writingFinished.resolve(undefined);
}There was a problem hiding this comment.
+1. it took some fiddling, but I think I got this working, + added a test
This lets us write our transform streams using async iterators, making them easier to read and understand, as well as consistently propagating errors. Blocked on DefinitelyTyped/DefinitelyTyped#16499 for compilation.
Also a couple more workarounds until DefinitelyTyped/DefinitelyTyped#16499 lands.
The `finish` event is fired when a writable stream can not be written to anymore. The `end` event is fired when a readable stream can not be read any more. When downstream of a readable stream, to tell that the stream is done you're interested in the `end` event, not the `finish` event.
rictic
force-pushed
the
async-transform-stream
branch
from
6040920
to
5be92d5
Compare
src/streams.ts
Outdated
| new Error( | ||
| `${this.constructor.name}` + | ||
| ` did not consume all input while transforming.`)); | ||
| this.push(null); |
There was a problem hiding this comment.
What's this push(null) about?
There was a problem hiding this comment.
It signals the end of the stream, that no more output will be written.
Added a comment.
This lets us write our transform streams using async iterators, making them easier to read and understand, as well as consistently propagating errors.