Implement polymer lint --watch
#928
Conversation
rictic
commented
Nov 16, 2017
rictic
commented
- CHANGELOG.md has been updated
rictic
force-pushed
the
lint-watch
branch
2 times, most recently
from
4416e32
to
0cc27dc
Compare
rictic
force-pushed
the
lint-watch
branch
3 times, most recently
from
6d5475c
to
2b60857
Compare
Introduces some logic to ensure that we react to the changes from lint fixes quickly, but without doing duplicated back-to-back runs, which would be very noisy for users (as well as being difficult to test).
src/lint/lint.ts
Outdated
| reportIfNoFix?: boolean; | ||
| } | ||
|
|
||
| async function runOnce( |
There was a problem hiding this comment.
+docs! also I'd prefer run instead of runOnce, I think run being singular is implied, especially if there's a second method that is explicitly "multiple/loop/watch/etc"
src/lint/lint.ts
Outdated
| for await(const changeBatch of watcher) { | ||
| await analyzer.filesChanged([...changeBatch]); | ||
|
|
||
| await runOnce( |
There was a problem hiding this comment.
oh, wait, do you pass a watcher to runOnce()? Can you document this?
src/lint/lint.ts
Outdated
| }); | ||
| } | ||
|
|
||
| private noticeChange(path: string) { |
src/lint/lint.ts
Outdated
| this.outOfBandNotices.add(path); | ||
| } | ||
|
|
||
| async * [Symbol.asyncIterator](): AsyncIterator<Set<string>> { |
|
All comments addressed! |
rictic
requested review from
FredKSchott
and removed request for
justinfagnani
|
[Justin's out this week, so passing this review over to Fred] |
| @@ -67,7 +67,7 @@ gulp.task( | |||
| })) | |||
| .pipe(tslint.report())); | |||
|
|
|||
| gulp.task('depcheck', () => { | |||
| gulp.task('depcheck', ['build'], () => { | |||
There was a problem hiding this comment.
why build first? it should be analyzing our typescript directly
There was a problem hiding this comment.
Without this change it crashes saying that chokidar is unused.
Adding some console.log statements around makes me think that it's unable to handle lint/lint.ts, giving a warning in the babylon parser, a little bit after an await statement. It seems like it's not handling typescript perfectly. Fortunately by ensuring that build happens first, it will also run over our generated JS, which it does handle correctly.
| * Run a single pass of the linter, and then report the results or fix warnings | ||
| * as requested by `options`. | ||
| * | ||
| * In a normal run this is called once and then it's done. When running with |
| new Set([...analysis.getFeatures({kind: 'document'})].map((d) => d.url)); | ||
| const watcher = new FilesystemChangeStream( | ||
| chokidar.watch([...paths], {persistent: true})); | ||
| for await (const changeBatch of watcher) { |
There was a problem hiding this comment.
man, for await is wild.
So the intention is that this iterator never completes, until the user kills the process, correct? That would be worth a comment, just to make sure future readers don't follow through past this.
There was a problem hiding this comment.
I know right!? It's the best.
I added a comment mentioning that this is a forever loop.
| @@ -183,6 +184,77 @@ Fixed 4 warnings. | |||
| </dom-module> | |||
| `); | |||
| }); | |||
|
|
|||
| suite('--watch', function() { | |||
