From 5679d0ad2e99fcc398645cdef6c7c9262af2d4ff Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sat, 24 Aug 2019 15:10:22 -0400 Subject: [PATCH 01/16] Watch files onbuild, even if build fails --- src/Module.ts | 1 + src/rollup/types.d.ts | 2 ++ src/watch/index.ts | 9 +++++++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index 4b2bdb9f7d4..e722473526c 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -244,6 +244,7 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; + props.graph = this.graph; let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index f24e305b4d7..b77e66a86ac 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -1,9 +1,11 @@ import * as ESTree from 'estree'; import { EventEmitter } from 'events'; +import Graph from '../Graph'; export const VERSION: string; export interface RollupError extends RollupLogProps { + graph: Graph; stack?: string; } diff --git a/src/watch/index.ts b/src/watch/index.ts index 5bd0284ee8e..1e9c34bb25b 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -9,6 +9,7 @@ import { OutputOptions, RollupBuild, RollupCache, + RollupError, RollupWatcher, WatcherOptions } from '../rollup/types'; @@ -100,7 +101,7 @@ export class Watcher { .catch(error => { this.running = false; this.emit('event', { - code: this.succeeded ? 'ERROR' : 'FATAL', + code: 'ERROR', error }); }) @@ -243,9 +244,13 @@ export class Task { result }); }) - .catch((error: Error) => { + .catch((error: RollupError) => { if (this.closed) return; + Array.from(error.graph.moduleById.keys()).forEach(id => { + this.watchFile(id); + }); + if (this.cache) { // this is necessary to ensure that any 'renamed' files // continue to be watched following an error From 7c003ac4bcd75bb44c850be9c8b46de6a56b4c1e Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sat, 24 Aug 2019 22:30:42 -0400 Subject: [PATCH 02/16] Add array, not graph, to error object --- src/Module.ts | 2 +- src/rollup/types.d.ts | 3 +-- src/watch/index.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index e722473526c..f6c11823250 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -244,7 +244,7 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - props.graph = this.graph; + props.watchFiles = Array.from(this.graph.moduleById.keys()); let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index b77e66a86ac..6fd7c2a8b88 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -1,12 +1,11 @@ import * as ESTree from 'estree'; import { EventEmitter } from 'events'; -import Graph from '../Graph'; export const VERSION: string; export interface RollupError extends RollupLogProps { - graph: Graph; stack?: string; + watchFiles: string[]; } export interface RollupWarning extends RollupLogProps { diff --git a/src/watch/index.ts b/src/watch/index.ts index 1e9c34bb25b..4920743e89b 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -247,7 +247,7 @@ export class Task { .catch((error: RollupError) => { if (this.closed) return; - Array.from(error.graph.moduleById.keys()).forEach(id => { + error.watchFiles.forEach(id => { this.watchFile(id); }); From 2162dc7a7ab65a424e8c83cd1ab71774295ccc70 Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 17:32:36 -0400 Subject: [PATCH 03/16] Update error format for tests --- src/Module.ts | 5 +++-- src/watch/index.ts | 11 ++++++++--- .../samples/cannot-call-external-namespace/_config.js | 1 + .../samples/cannot-call-internal-namespace/_config.js | 1 + .../samples/default-not-reexported/_config.js | 5 +++++ .../function/samples/double-default-export/_config.js | 1 + test/function/samples/double-named-export/_config.js | 1 + .../function/samples/double-named-reexport/_config.js | 1 + .../samples/duplicate-import-fails/_config.js | 1 + .../duplicate-import-specifier-fails/_config.js | 1 + .../_config.js | 1 + test/function/samples/error-parse-json/_config.js | 1 + .../samples/error-parse-unknown-extension/_config.js | 1 + .../samples/export-not-at-top-level-fails/_config.js | 1 + .../samples/import-not-at-top-level-fails/_config.js | 1 + .../samples/import-of-unexported-fails/_config.js | 1 + .../namespace-reassign-import-fails/_config.js | 1 + .../samples/namespace-update-import-fails/_config.js | 1 + .../function/samples/reassign-import-fails/_config.js | 1 + .../reassign-import-not-at-top-level-fails/_config.js | 1 + .../samples/reexport-missing-error/_config.js | 1 + .../update-expression-of-import-fails/_config.js | 1 + 22 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index f6c11823250..0c733bc32d8 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -44,7 +44,7 @@ import { error } from './utils/error'; import getCodeFrame from './utils/getCodeFrame'; import { getOriginalLocation } from './utils/getOriginalLocation'; import { makeLegal } from './utils/identifierHelpers'; -import { basename, extname } from './utils/path'; +import { basename, extname, isAbsolute } from './utils/path'; import { markPureCallExpressions } from './utils/pureComments'; import relativeId from './utils/relativeId'; import { RenderOptions } from './utils/renderHelpers'; @@ -244,7 +244,8 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - props.watchFiles = Array.from(this.graph.moduleById.keys()); + // Only consider absolute paths watchFiles since sometimes modules may be caught up in this + props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/watch/index.ts b/src/watch/index.ts index 4920743e89b..ee3bb017013 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -247,9 +247,14 @@ export class Task { .catch((error: RollupError) => { if (this.closed) return; - error.watchFiles.forEach(id => { - this.watchFile(id); - }); + if (this.watched.size === 0) { + const watched = (this.watched = new Set()); + + error.watchFiles.forEach(id => { + watched.add(id); + this.watchFile(id); + }); + } if (this.cache) { // this is necessary to ensure that any 'renamed' files diff --git a/test/function/samples/cannot-call-external-namespace/_config.js b/test/function/samples/cannot-call-external-namespace/_config.js index ab64d978213..1dd8124c147 100644 --- a/test/function/samples/cannot-call-external-namespace/_config.js +++ b/test/function/samples/cannot-call-external-namespace/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'CANNOT_CALL_NAMESPACE', message: `Cannot call a namespace ('foo')`, pos: 28, + watchFiles: [path.resolve(__dirname, 'main.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 2, diff --git a/test/function/samples/cannot-call-internal-namespace/_config.js b/test/function/samples/cannot-call-internal-namespace/_config.js index 150daa58458..a75a74e7f5d 100644 --- a/test/function/samples/cannot-call-internal-namespace/_config.js +++ b/test/function/samples/cannot-call-internal-namespace/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'CANNOT_CALL_NAMESPACE', message: `Cannot call a namespace ('foo')`, pos: 33, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 2, diff --git a/test/function/samples/default-not-reexported/_config.js b/test/function/samples/default-not-reexported/_config.js index dc44615c8b5..947141dbbbb 100644 --- a/test/function/samples/default-not-reexported/_config.js +++ b/test/function/samples/default-not-reexported/_config.js @@ -6,6 +6,11 @@ module.exports = { code: 'MISSING_EXPORT', message: `'default' is not exported by foo.js`, pos: 7, + watchFiles: [ + path.resolve(__dirname, 'main.js'), + path.resolve(__dirname, 'foo.js'), + path.resolve(__dirname, 'bar.js') + ], loc: { file: path.resolve(__dirname, 'main.js'), line: 1, diff --git a/test/function/samples/double-default-export/_config.js b/test/function/samples/double-default-export/_config.js index e8d9850f9bb..c1a58edbcc7 100644 --- a/test/function/samples/double-default-export/_config.js +++ b/test/function/samples/double-default-export/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `Duplicate export 'default'`, pos: 25, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'foo.js'), line: 2, diff --git a/test/function/samples/double-named-export/_config.js b/test/function/samples/double-named-export/_config.js index 8fda67d1a65..246fd8efee6 100644 --- a/test/function/samples/double-named-export/_config.js +++ b/test/function/samples/double-named-export/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `Duplicate export 'foo'`, pos: 38, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'foo.js'), line: 3, diff --git a/test/function/samples/double-named-reexport/_config.js b/test/function/samples/double-named-reexport/_config.js index b20d5417941..c8a2cb35685 100644 --- a/test/function/samples/double-named-reexport/_config.js +++ b/test/function/samples/double-named-reexport/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `Duplicate export 'foo'`, pos: 38, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'foo.js'), line: 3, diff --git a/test/function/samples/duplicate-import-fails/_config.js b/test/function/samples/duplicate-import-fails/_config.js index 6367ad51ede..b631dd140a0 100644 --- a/test/function/samples/duplicate-import-fails/_config.js +++ b/test/function/samples/duplicate-import-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `Identifier 'a' has already been declared`, pos: 36, + watchFiles: [path.resolve(__dirname, 'main.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 2, diff --git a/test/function/samples/duplicate-import-specifier-fails/_config.js b/test/function/samples/duplicate-import-specifier-fails/_config.js index 3fe14caf093..0ec688bf439 100644 --- a/test/function/samples/duplicate-import-specifier-fails/_config.js +++ b/test/function/samples/duplicate-import-specifier-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `Identifier 'a' has already been declared`, pos: 12, + watchFiles: [path.resolve(__dirname, 'main.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 1, diff --git a/test/function/samples/error-after-transform-should-throw-correct-location/_config.js b/test/function/samples/error-after-transform-should-throw-correct-location/_config.js index 1fe521ec6f4..23001b3ba56 100644 --- a/test/function/samples/error-after-transform-should-throw-correct-location/_config.js +++ b/test/function/samples/error-after-transform-should-throw-correct-location/_config.js @@ -22,6 +22,7 @@ module.exports = { code: 'MISSING_EXPORT', message: `'default' is not exported by empty.js`, pos: 44, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 1, diff --git a/test/function/samples/error-parse-json/_config.js b/test/function/samples/error-parse-json/_config.js index 7d23ff6baa1..b0974b84a66 100644 --- a/test/function/samples/error-parse-json/_config.js +++ b/test/function/samples/error-parse-json/_config.js @@ -7,6 +7,7 @@ module.exports = { code: 'PARSE_ERROR', message: 'Unexpected token (Note that you need rollup-plugin-json to import JSON files)', pos: 10, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'file.json')], loc: { file: path.resolve(__dirname, 'file.json'), line: 2, diff --git a/test/function/samples/error-parse-unknown-extension/_config.js b/test/function/samples/error-parse-unknown-extension/_config.js index 5ff9e767a8f..15659ced761 100644 --- a/test/function/samples/error-parse-unknown-extension/_config.js +++ b/test/function/samples/error-parse-unknown-extension/_config.js @@ -8,6 +8,7 @@ module.exports = { message: 'Unexpected token (Note that you need plugins to import files that are not JavaScript)', pos: 0, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'file.css')], loc: { file: path.resolve(__dirname, 'file.css'), line: 1, diff --git a/test/function/samples/export-not-at-top-level-fails/_config.js b/test/function/samples/export-not-at-top-level-fails/_config.js index f324166ae0e..657b4222cd3 100644 --- a/test/function/samples/export-not-at-top-level-fails/_config.js +++ b/test/function/samples/export-not-at-top-level-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `'import' and 'export' may only appear at the top level`, pos: 19, + watchFiles: [path.resolve(__dirname, 'main.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 2, diff --git a/test/function/samples/import-not-at-top-level-fails/_config.js b/test/function/samples/import-not-at-top-level-fails/_config.js index 4897343cbcb..46b1434602a 100644 --- a/test/function/samples/import-not-at-top-level-fails/_config.js +++ b/test/function/samples/import-not-at-top-level-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'PARSE_ERROR', message: `'import' and 'export' may only appear at the top level`, pos: 19, + watchFiles: [path.resolve(__dirname, 'main.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 2, diff --git a/test/function/samples/import-of-unexported-fails/_config.js b/test/function/samples/import-of-unexported-fails/_config.js index e75f5c6e7cc..0db25ac0ad5 100644 --- a/test/function/samples/import-of-unexported-fails/_config.js +++ b/test/function/samples/import-of-unexported-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'MISSING_EXPORT', message: `'default' is not exported by empty.js`, pos: 7, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 1, diff --git a/test/function/samples/namespace-reassign-import-fails/_config.js b/test/function/samples/namespace-reassign-import-fails/_config.js index 9c0b614ee5f..3ab81beb7bd 100644 --- a/test/function/samples/namespace-reassign-import-fails/_config.js +++ b/test/function/samples/namespace-reassign-import-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'ILLEGAL_NAMESPACE_REASSIGNMENT', message: `Illegal reassignment to import 'exp'`, pos: 31, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 3, diff --git a/test/function/samples/namespace-update-import-fails/_config.js b/test/function/samples/namespace-update-import-fails/_config.js index 43276c1e756..f5d608e7ffd 100644 --- a/test/function/samples/namespace-update-import-fails/_config.js +++ b/test/function/samples/namespace-update-import-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'ILLEGAL_NAMESPACE_REASSIGNMENT', message: `Illegal reassignment to import 'exp'`, pos: 31, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 3, diff --git a/test/function/samples/reassign-import-fails/_config.js b/test/function/samples/reassign-import-fails/_config.js index b1e552ca243..1f8b3bf9027 100644 --- a/test/function/samples/reassign-import-fails/_config.js +++ b/test/function/samples/reassign-import-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'ILLEGAL_REASSIGNMENT', message: `Illegal reassignment to import 'x'`, pos: 113, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 8, diff --git a/test/function/samples/reassign-import-not-at-top-level-fails/_config.js b/test/function/samples/reassign-import-not-at-top-level-fails/_config.js index 4d12a57193e..e826b171f13 100644 --- a/test/function/samples/reassign-import-not-at-top-level-fails/_config.js +++ b/test/function/samples/reassign-import-not-at-top-level-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'ILLEGAL_REASSIGNMENT', message: `Illegal reassignment to import 'x'`, pos: 95, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 7, diff --git a/test/function/samples/reexport-missing-error/_config.js b/test/function/samples/reexport-missing-error/_config.js index dc00c39b6f3..032dc7b3c7e 100644 --- a/test/function/samples/reexport-missing-error/_config.js +++ b/test/function/samples/reexport-missing-error/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'MISSING_EXPORT', message: `'foo' is not exported by empty.js`, pos: 9, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 1, diff --git a/test/function/samples/update-expression-of-import-fails/_config.js b/test/function/samples/update-expression-of-import-fails/_config.js index 64bccf4f1f7..cff3fb53b1d 100644 --- a/test/function/samples/update-expression-of-import-fails/_config.js +++ b/test/function/samples/update-expression-of-import-fails/_config.js @@ -6,6 +6,7 @@ module.exports = { code: 'ILLEGAL_REASSIGNMENT', message: `Illegal reassignment to import 'a'`, pos: 28, + watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')], loc: { file: path.resolve(__dirname, 'main.js'), line: 3, From a81204eccd532cb3f683631d94cd45cc8448b944 Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 17:39:05 -0400 Subject: [PATCH 04/16] Update watch tests to throw ERRORs, not FATAL --- test/watch/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/watch/index.js b/test/watch/index.js index 1af42233e9e..1124c430215 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -1054,7 +1054,7 @@ describe('rollup.watch', () => { return sequence(watcher, [ 'START', 'BUNDLE_START', - 'FATAL', + 'ERROR', event => { assert.ok(event.error.message.startsWith('Transform dependency')); assert.ok(event.error.message.endsWith('does not exist.')); From d3634fdd68a2fa7663b749a84051c8b8e60752cc Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 17:56:33 -0400 Subject: [PATCH 05/16] Add test for recovering from error on watch --- test/watch/index.js | 33 ++++++++++++++++++++++++++++++++ test/watch/samples/error/main.js | 2 ++ 2 files changed, 35 insertions(+) create mode 100644 test/watch/samples/error/main.js diff --git a/test/watch/index.js b/test/watch/index.js index 1124c430215..2dd9737ee07 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -281,6 +281,39 @@ describe('rollup.watch', () => { }); }); + it('recovers from an error on initial build', () => { + return sander + .copydir('test/watch/samples/error') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'ERROR', + () => { + assert.strictEqual(sander.existsSync('../_tmp/output/bundle.js'), false); + sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); + }, + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 43); + } + ]); + }); + }); + it('recovers from an error even when erroring file was "renamed" (#38)', () => { return sander .copydir('test/watch/samples/basic') diff --git a/test/watch/samples/error/main.js b/test/watch/samples/error/main.js new file mode 100644 index 00000000000..2909584a13a --- /dev/null +++ b/test/watch/samples/error/main.js @@ -0,0 +1,2 @@ +export default 42; += From e825a4466e7ee9ec8d352065be45ab7118424a17 Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 19:44:27 -0400 Subject: [PATCH 06/16] Fix eslint-utils security vulnerability --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2d4ed90320b..ea09d24fd56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1614,9 +1614,9 @@ } }, "eslint-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/eslint-utils/-/eslint-utils-1.4.0.tgz", - "integrity": "sha512-7ehnzPaP5IIEh1r1tkjuIrxqhNkzUJa9z3R92tLJdZIVdWaczEhr3EbhGtsMrVxi1KeR8qA7Off6SWc5WNQqyQ==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/eslint-utils/-/eslint-utils-1.4.2.tgz", + "integrity": "sha512-eAZS2sEUMlIeCjBeubdj45dmBHQwPHWyBcT1VSYB7o9x9WRRqKxyUoiXlRjyAwzN7YEzHJlYg0NmzDRWx6GP4Q==", "dev": true, "requires": { "eslint-visitor-keys": "^1.0.0" From 830e16b13f40309e362a66939193e37d49319a8b Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 20:06:55 -0400 Subject: [PATCH 07/16] More restrictive object testing, ts fixes --- src/Module.ts | 10 ++++++++-- src/rollup/types.d.ts | 2 +- src/watch/index.ts | 4 +--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index 0c733bc32d8..aeb3a1a6cb1 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -244,8 +244,14 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - // Only consider absolute paths watchFiles since sometimes modules may be caught up in this - props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); + if (this.graph && typeof this.graph.moduleById === 'object') { + try { + // Only consider absolute paths watchFiles since sometimes modules may be caught up in this + props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); + } catch { + props.watchFiles = []; + } + } let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 6fd7c2a8b88..2e3fd1f1aa5 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -5,7 +5,7 @@ export const VERSION: string; export interface RollupError extends RollupLogProps { stack?: string; - watchFiles: string[]; + watchFiles?: string[]; } export interface RollupWarning extends RollupLogProps { diff --git a/src/watch/index.ts b/src/watch/index.ts index ee3bb017013..6ac63f57fc9 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -26,7 +26,6 @@ export class Watcher { private invalidatedIds: Set = new Set(); private rerun = false; private running: boolean; - private succeeded = false; private tasks: Task[]; constructor(configs: GenericConfigObject[] | GenericConfigObject) { @@ -91,7 +90,6 @@ export class Watcher { for (const task of this.tasks) taskPromise = taskPromise.then(() => task.run()); return taskPromise .then(() => { - this.succeeded = true; this.running = false; this.emit('event', { @@ -247,7 +245,7 @@ export class Task { .catch((error: RollupError) => { if (this.closed) return; - if (this.watched.size === 0) { + if (this.watched.size === 0 && error.watchFiles) { const watched = (this.watched = new Set()); error.watchFiles.forEach(id => { From edb85c27f2f622de661bf57781ae0aae49ea41ff Mon Sep 17 00:00:00 2001 From: mhkeller Date: Sun, 25 Aug 2019 22:15:12 -0400 Subject: [PATCH 08/16] Better type checking --- src/Module.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index aeb3a1a6cb1..312db3de090 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -244,13 +244,9 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - if (this.graph && typeof this.graph.moduleById === 'object') { - try { - // Only consider absolute paths watchFiles since sometimes modules may be caught up in this - props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); - } catch { - props.watchFiles = []; - } + if (this.graph && this.graph.moduleById instanceof Map === true) { + // Only consider absolute paths watchFiles since sometimes modules may be caught up in this + props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); } let location = locate(this.code, pos, { offsetLine: 1 }); From d711ecf38eda25c5b3c4f00be7cf063b4e166115 Mon Sep 17 00:00:00 2001 From: mhkeller Date: Mon, 26 Aug 2019 10:35:59 -0400 Subject: [PATCH 09/16] Better type checking --- src/Module.ts | 6 ++---- src/watch/index.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index 9eb6993e08a..6b35ee6c85f 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -244,10 +244,8 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - if (this.graph && this.graph.moduleById instanceof Map === true) { - // Only consider absolute paths watchFiles since sometimes modules may be caught up in this - props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); - } + // Only consider absolute paths watchFiles since sometimes modules may be caught up in this + props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/watch/index.ts b/src/watch/index.ts index 6ac63f57fc9..0cabdc964ea 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -245,7 +245,7 @@ export class Task { .catch((error: RollupError) => { if (this.closed) return; - if (this.watched.size === 0 && error.watchFiles) { + if (this.watched.size === 0 && Array.isArray(error.watchFiles)) { const watched = (this.watched = new Set()); error.watchFiles.forEach(id => { From 0cad1b9fa6de283583e5a93a3003956816c839d6 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 10:29:20 +0200 Subject: [PATCH 10/16] Improve watch behaviour * Do not add files to Graph.watchFiles after all files have been loaded but immediately when starting to load them so that the same watchFile logic can be used for successful and unsuccessful builds (and files added by plugins are not ignored on errors) * Do not unwatch files that have been removed if an error occurred but only when a build has succeeded * Do not re-watch files of the previous build when an error occurred as those are still being watched; just additionally watch any files attached to the error * Make sure that watching a file automatically adds it to the list of "watched" files and only reset this list when clearing unneeded files * Remove logic to filter for absolute ids for now to restore the old behaviour * Replace some .forEach with for loops for slightly improved performance --- src/Graph.ts | 1 - src/Module.ts | 5 +- src/ModuleLoader.ts | 1 + src/watch/fileWatchers.ts | 4 +- src/watch/index.ts | 87 ++++++++------------ test/watch/index.js | 114 +++++++++++++++++++++++--- test/watch/samples/dependency/dep.js | 1 + test/watch/samples/dependency/main.js | 2 + 8 files changed, 146 insertions(+), 69 deletions(-) create mode 100644 test/watch/samples/dependency/dep.js create mode 100644 test/watch/samples/dependency/main.js diff --git a/src/Graph.ts b/src/Graph.ts index b210a13ba92..1cc3e41c2ac 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -222,7 +222,6 @@ export default class Graph { for (const module of this.moduleById.values()) { if (module instanceof Module) { this.modules.push(module); - this.watchFiles[module.id] = true; } else { this.externalModules.push(module); } diff --git a/src/Module.ts b/src/Module.ts index 6b35ee6c85f..0856531061a 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -44,7 +44,7 @@ import { error } from './utils/error'; import getCodeFrame from './utils/getCodeFrame'; import { getOriginalLocation } from './utils/getOriginalLocation'; import { makeLegal } from './utils/identifierHelpers'; -import { basename, extname, isAbsolute } from './utils/path'; +import { basename, extname } from './utils/path'; import { markPureCallExpressions } from './utils/pureComments'; import relativeId from './utils/relativeId'; import { RenderOptions } from './utils/renderHelpers'; @@ -244,8 +244,7 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - // Only consider absolute paths watchFiles since sometimes modules may be caught up in this - props.watchFiles = Array.from(this.graph.moduleById.keys()).filter(d => isAbsolute(d)); + props.watchFiles = Object.keys(this.graph.watchFiles); let location = locate(this.code, pos, { offsetLine: 1 }); try { diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index c03014ade55..193b1fce733 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -280,6 +280,7 @@ export class ModuleLoader { const module: Module = new Module(this.graph, id, moduleSideEffects, isEntry); this.modulesById.set(id, module); + this.graph.watchFiles[id] = true; const manualChunkAlias = this.getManualChunk(id); if (typeof manualChunkAlias === 'string') { this.addModuleToManualChunk(manualChunkAlias, module); diff --git a/src/watch/fileWatchers.ts b/src/watch/fileWatchers.ts index 97c8084694b..3fb35ec3664 100644 --- a/src/watch/fileWatchers.ts +++ b/src/watch/fileWatchers.ts @@ -60,9 +60,9 @@ export default class FileWatcher { const handleWatchEvent = (event: string) => { if (event === 'rename' || event === 'unlink') { - this.close(); - group.delete(id); + modifiedTime = -1; this.trigger(id); + return; } else { let stats: fs.Stats; try { diff --git a/src/watch/index.ts b/src/watch/index.ts index 0cabdc964ea..592b3511b3e 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -48,9 +48,9 @@ export class Watcher { close() { if (this.buildTimeout) clearTimeout(this.buildTimeout); - this.tasks.forEach(task => { + for (const task of this.tasks) { task.close(); - }); + } this.emitter.removeAllListeners(); } @@ -72,7 +72,9 @@ export class Watcher { this.buildTimeout = setTimeout(() => { this.buildTimeout = null; - this.invalidatedIds.forEach(id => this.emit('change', id)); + for (const id of this.invalidatedIds) { + this.emit('change', id); + } this.invalidatedIds.clear(); this.emit('restart'); this.run(); @@ -171,20 +173,20 @@ export class Task { close() { this.closed = true; - this.watched.forEach(id => { + for (const id of this.watched) { deleteTask(id, this, this.chokidarOptionsHash); - }); + } } invalidate(id: string, isTransformDependency: boolean) { this.invalidated = true; if (isTransformDependency) { - (this.cache.modules as ModuleJSON[]).forEach(module => { + for (const module of this.cache.modules as ModuleJSON[]) { if (!module.transformDependencies || module.transformDependencies.indexOf(id) === -1) return; // effective invalidation module.originalCode = null as any; - }); + } } this.watcher.invalidate(id); } @@ -210,27 +212,7 @@ export class Task { return rollup(options) .then(result => { if (this.closed) return undefined as any; - const previouslyWatched = this.watched; - const watched = (this.watched = new Set()); - - this.cache = result.cache; - this.watchFiles = result.watchFiles; - for (const module of this.cache.modules as ModuleJSON[]) { - if (module.transformDependencies) { - module.transformDependencies.forEach(depId => { - watched.add(depId); - this.watchFile(depId, true); - }); - } - } - for (const id of this.watchFiles) { - watched.add(id); - this.watchFile(id); - } - for (const id of previouslyWatched) { - if (!watched.has(id)) deleteTask(id, this, this.chokidarOptionsHash); - } - + this.updateWatchedFiles(result); return Promise.all(this.outputs.map(output => result.write(output))).then(() => result); }) .then((result: RollupBuild) => { @@ -245,37 +227,40 @@ export class Task { .catch((error: RollupError) => { if (this.closed) return; - if (this.watched.size === 0 && Array.isArray(error.watchFiles)) { - const watched = (this.watched = new Set()); - - error.watchFiles.forEach(id => { - watched.add(id); + if (Array.isArray(error.watchFiles)) { + for (const id of error.watchFiles) { this.watchFile(id); - }); - } - - if (this.cache) { - // this is necessary to ensure that any 'renamed' files - // continue to be watched following an error - if (this.cache.modules) { - this.cache.modules.forEach(module => { - if (module.transformDependencies) { - module.transformDependencies.forEach(depId => { - this.watchFile(depId, true); - }); - } - }); } - this.watchFiles.forEach(id => { - this.watchFile(id); - }); } throw error; }); } - watchFile(id: string, isTransformDependency = false) { + private updateWatchedFiles(result: RollupBuild) { + const previouslyWatched = this.watched; + this.watched = new Set(); + this.watchFiles = result.watchFiles; + this.cache = result.cache; + for (const id of this.watchFiles) { + this.watchFile(id); + } + if (this.cache.modules) { + for (const module of this.cache.modules) { + if (module.transformDependencies) { + for (const depId of module.transformDependencies) { + this.watchFile(depId, true); + } + } + } + } + for (const id of previouslyWatched) { + if (!this.watched.has(id)) deleteTask(id, this, this.chokidarOptionsHash); + } + } + + private watchFile(id: string, isTransformDependency = false) { if (!this.filter(id)) return; + this.watched.add(id); if (this.outputFiles.some(file => file === id)) { throw new Error('Cannot import the generated bundle'); diff --git a/test/watch/index.js b/test/watch/index.js index 2dd9737ee07..9423b60f215 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -356,6 +356,86 @@ describe('rollup.watch', () => { }); }); + it('recovers from an error even when an entry file was deleted and recreated', () => { + return sander + .copydir('test/watch/samples/basic') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 42); + sander.unlinkSync('test/_tmp/input/main.js'); + }, + 'START', + 'BUNDLE_START', + 'ERROR', + () => { + sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); + }, + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 43); + } + ]); + }); + }); + + it('stops watching files that are no longer part of the graph', () => { + return sander + .copydir('test/watch/samples/dependency') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 43); + sander.writeFileSync('test/_tmp/input/main.js', 'export default 42;'); + }, + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 42); + let unexpectedEvent = false; + watcher.once('event', event => { + unexpectedEvent = event; + }); + sander.writeFileSync('test/_tmp/input/dep.js', '= invalid'); + return wait(400).then(() => assert.strictEqual(unexpectedEvent, false)); + } + ]); + }); + }); + it('refuses to watch the output file (#15)', () => { return sander .copydir('test/watch/samples/basic') @@ -435,12 +515,17 @@ describe('rollup.watch', () => { foo: 'foo-2', bar: 'bar-1' }); - sander.writeFileSync('test/_tmp/input/bar.js', `export default 'bar-2';`); - }, - () => { - assert.deepStrictEqual(run('../_tmp/output/bundle.js'), { - foo: 'foo-2', - bar: 'bar-1' + let unexpectedEvent = false; + watcher.once('event', event => { + unexpectedEvent = event; + }); + sander.writeFileSync('test/_tmp/input/bar.js', "export default 'bar-2';"); + return wait(400).then(() => { + assert.deepStrictEqual(run('../_tmp/output/bundle.js'), { + foo: 'foo-2', + bar: 'bar-1' + }); + assert.strictEqual(unexpectedEvent, false); }); } ]); @@ -485,12 +570,17 @@ describe('rollup.watch', () => { foo: 'foo-2', bar: 'bar-1' }); - sander.writeFileSync('test/_tmp/input/bar.js', `export default 'bar-2';`); - }, - () => { - assert.deepStrictEqual(run('../_tmp/output/bundle.js'), { - foo: 'foo-2', - bar: 'bar-1' + let unexpectedEvent = false; + watcher.once('event', event => { + unexpectedEvent = event; + }); + sander.writeFileSync('test/_tmp/input/bar.js', "export default 'bar-2';"); + return wait(400).then(() => { + assert.deepStrictEqual(run('../_tmp/output/bundle.js'), { + foo: 'foo-2', + bar: 'bar-1' + }); + assert.strictEqual(unexpectedEvent, false); }); } ]); diff --git a/test/watch/samples/dependency/dep.js b/test/watch/samples/dependency/dep.js new file mode 100644 index 00000000000..46d3ca8c61f --- /dev/null +++ b/test/watch/samples/dependency/dep.js @@ -0,0 +1 @@ +export const value = 42; diff --git a/test/watch/samples/dependency/main.js b/test/watch/samples/dependency/main.js new file mode 100644 index 00000000000..30ce9a1c47e --- /dev/null +++ b/test/watch/samples/dependency/main.js @@ -0,0 +1,2 @@ +import { value } from './dep.js'; +export default value + 1; From 20a7944e2db5dd2c3b6524853591933bb1f65509 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 10:40:04 +0200 Subject: [PATCH 11/16] Add watchFiles to errors not only when an explicit file position is given but for any error that occurs during the build phase when there are already some watchFiles --- src/Module.ts | 10 ++++++++-- .../internal-reexports-from-external/_config.js | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index 0856531061a..abd9153ad74 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -40,6 +40,7 @@ import { RollupWarning, TransformModuleJSON } from './rollup/types'; +import { BuildPhase } from './utils/buildPhase'; import { error } from './utils/error'; import getCodeFrame from './utils/getCodeFrame'; import { getOriginalLocation } from './utils/getOriginalLocation'; @@ -244,8 +245,6 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - props.watchFiles = Object.keys(this.graph.watchFiles); - let location = locate(this.code, pos, { offsetLine: 1 }); try { location = getOriginalLocation(this.sourcemapChain, location); @@ -273,6 +272,13 @@ export default class Module { props.frame = getCodeFrame(this.originalCode, location.line, location.column); } + if (this.graph.phase < BuildPhase.GENERATE) { + const watchFiles = Object.keys(this.graph.watchFiles); + if (watchFiles.length > 0) { + props.watchFiles = watchFiles; + } + } + error(props); } diff --git a/test/function/samples/internal-reexports-from-external/_config.js b/test/function/samples/internal-reexports-from-external/_config.js index 4b56fa1782d..f6c6ebd8d05 100644 --- a/test/function/samples/internal-reexports-from-external/_config.js +++ b/test/function/samples/internal-reexports-from-external/_config.js @@ -10,6 +10,7 @@ module.exports = { code: 'NAMESPACE_CANNOT_CONTAIN_EXTERNAL', message: 'Cannot create an explicit namespace object for module "reexport" because it contains a reexported external namespace', - id: path.join(__dirname, 'reexport.js') + id: path.join(__dirname, 'reexport.js'), + watchFiles: [path.join(__dirname, 'main.js'), path.join(__dirname, 'reexport.js')] } }; From 5a242d2ba4ca816da32f023f9850a08cfe4c3dc2 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 11:44:17 +0200 Subject: [PATCH 12/16] Only keep watching deleted files when using chokidar --- src/watch/fileWatchers.ts | 7 +++- test/watch/index.js | 78 ++++++++++++++++++++------------------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/watch/fileWatchers.ts b/src/watch/fileWatchers.ts index 3fb35ec3664..756cdd7262c 100644 --- a/src/watch/fileWatchers.ts +++ b/src/watch/fileWatchers.ts @@ -60,7 +60,12 @@ export default class FileWatcher { const handleWatchEvent = (event: string) => { if (event === 'rename' || event === 'unlink') { - modifiedTime = -1; + if (chokidarOptions) { + modifiedTime = -1; + } else { + this.close(); + group.delete(id); + } this.trigger(id); return; } else { diff --git a/test/watch/index.js b/test/watch/index.js index 9423b60f215..3a35c1ac47d 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -356,45 +356,47 @@ describe('rollup.watch', () => { }); }); - it('recovers from an error even when an entry file was deleted and recreated', () => { - return sander - .copydir('test/watch/samples/basic') - .to('test/_tmp/input') - .then(() => { - const watcher = rollup.watch({ - input: 'test/_tmp/input/main.js', - output: { - file: 'test/_tmp/output/bundle.js', - format: 'cjs' - }, - watch: { chokidar } - }); + if (chokidar) { + it('recovers from an error even when an entry file was deleted and recreated', () => { + return sander + .copydir('test/watch/samples/basic') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); - return sequence(watcher, [ - 'START', - 'BUNDLE_START', - 'BUNDLE_END', - 'END', - () => { - assert.strictEqual(run('../_tmp/output/bundle.js'), 42); - sander.unlinkSync('test/_tmp/input/main.js'); - }, - 'START', - 'BUNDLE_START', - 'ERROR', - () => { - sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); - }, - 'START', - 'BUNDLE_START', - 'BUNDLE_END', - 'END', - () => { - assert.strictEqual(run('../_tmp/output/bundle.js'), 43); - } - ]); - }); - }); + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 42); + sander.unlinkSync('test/_tmp/input/main.js'); + }, + 'START', + 'BUNDLE_START', + 'ERROR', + () => { + sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); + }, + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 43); + } + ]); + }); + }); + } it('stops watching files that are no longer part of the graph', () => { return sander From 96c8975612da613a29645e3111a33e4137ce8960 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 13:02:19 +0200 Subject: [PATCH 13/16] Improve coverage and remove unused edge cases --- src/Graph.ts | 2 +- src/Module.ts | 11 ++--- src/rollup/types.d.ts | 4 +- src/watch/index.ts | 19 +++----- test/watch/index.js | 100 +++++++++++++++++++++++++++++++++++++----- 5 files changed, 101 insertions(+), 35 deletions(-) diff --git a/src/Graph.ts b/src/Graph.ts index 1cc3e41c2ac..6ad883f4110 100644 --- a/src/Graph.ts +++ b/src/Graph.ts @@ -330,7 +330,7 @@ export default class Graph { return { modules: this.modules.map(module => module.toJSON()), plugins: this.pluginCache - } as any; + }; } includeMarked(modules: Module[]) { diff --git a/src/Module.ts b/src/Module.ts index abd9153ad74..ab77c688548 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -40,7 +40,6 @@ import { RollupWarning, TransformModuleJSON } from './rollup/types'; -import { BuildPhase } from './utils/buildPhase'; import { error } from './utils/error'; import getCodeFrame from './utils/getCodeFrame'; import { getOriginalLocation } from './utils/getOriginalLocation'; @@ -219,7 +218,7 @@ export default class Module { private graph: Graph; private magicString!: MagicString; private namespaceVariable: NamespaceVariable = undefined as any; - private transformDependencies: string[] | null = null; + private transformDependencies: string[] = []; private transitiveReexports?: string[]; constructor(graph: Graph, id: string, moduleSideEffects: boolean, isEntry: boolean) { @@ -272,11 +271,9 @@ export default class Module { props.frame = getCodeFrame(this.originalCode, location.line, location.column); } - if (this.graph.phase < BuildPhase.GENERATE) { - const watchFiles = Object.keys(this.graph.watchFiles); - if (watchFiles.length > 0) { - props.watchFiles = watchFiles; - } + const watchFiles = Object.keys(this.graph.watchFiles); + if (watchFiles.length > 0) { + props.watchFiles = watchFiles; } error(props); diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 2e3fd1f1aa5..850e5c8136f 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -106,7 +106,7 @@ export interface TransformModuleJSON { originalSourcemap: ExistingDecodedSourceMap | null; resolvedIds?: ResolvedIdMap; sourcemapChain: DecodedSourceMapOrMissing[]; - transformDependencies: string[] | null; + transformDependencies: string[]; } export interface ModuleJSON extends TransformModuleJSON { @@ -528,7 +528,7 @@ export interface SerializablePluginCache { } export interface RollupCache { - modules?: ModuleJSON[]; + modules: ModuleJSON[]; plugins?: Record; } diff --git a/src/watch/index.ts b/src/watch/index.ts index 592b3511b3e..5f9a3e71be1 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -5,7 +5,6 @@ import createFilter from 'rollup-pluginutils/src/createFilter'; import rollup, { setWatcher } from '../rollup/index'; import { InputOptions, - ModuleJSON, OutputOptions, RollupBuild, RollupCache, @@ -115,7 +114,7 @@ export class Watcher { } export class Task { - cache: RollupCache; + cache: RollupCache = { modules: [] }; watchFiles: string[] = []; private chokidarOptions: WatchOptions; @@ -130,7 +129,6 @@ export class Task { private watcher: Watcher; constructor(watcher: Watcher, config: GenericConfigObject) { - this.cache = null as any; this.watcher = watcher; this.closed = false; @@ -181,9 +179,8 @@ export class Task { invalidate(id: string, isTransformDependency: boolean) { this.invalidated = true; if (isTransformDependency) { - for (const module of this.cache.modules as ModuleJSON[]) { - if (!module.transformDependencies || module.transformDependencies.indexOf(id) === -1) - return; + for (const module of this.cache.modules) { + if (module.transformDependencies.indexOf(id) === -1) return; // effective invalidation module.originalCode = null as any; } @@ -244,13 +241,9 @@ export class Task { for (const id of this.watchFiles) { this.watchFile(id); } - if (this.cache.modules) { - for (const module of this.cache.modules) { - if (module.transformDependencies) { - for (const depId of module.transformDependencies) { - this.watchFile(depId, true); - } - } + for (const module of this.cache.modules) { + for (const depId of module.transformDependencies) { + this.watchFile(depId, true); } } for (const id of previouslyWatched) { diff --git a/test/watch/index.js b/test/watch/index.js index 3a35c1ac47d..56f065858f5 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -11,7 +11,7 @@ function wait(ms) { }); } -describe('rollup.watch', () => { +describe.only('rollup.watch', () => { beforeEach(() => { process.chdir(cwd); return sander.rimraf('test/_tmp'); @@ -64,11 +64,9 @@ describe('rollup.watch', () => { runTests(false); }); - if (!process.env.CI) { - describe('chokidar', () => { - runTests(true); - }); - } + describe('chokidar', () => { + runTests(true); + }); function runTests(chokidar) { it('watches a file', () => { @@ -398,6 +396,70 @@ describe('rollup.watch', () => { }); } + it('handles closing the watcher during a build', () => { + return sander + .copydir('test/watch/samples/basic') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + setTimeout(() => watcher.close(), 50); + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + () => { + sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); + let unexpectedEvent = false; + watcher.once('event', event => { + unexpectedEvent = event; + }); + sander.writeFileSync('test/_tmp/input/dep.js', '= invalid'); + return wait(400).then(() => assert.strictEqual(unexpectedEvent, false)); + } + ]); + }); + }); + + it('handles closing the watcher during a build even if an error occurred', () => { + return sander + .copydir('test/watch/samples/error') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + setTimeout(() => watcher.close(), 50); + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'ERROR', + () => { + sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); + let unexpectedEvent = false; + watcher.once('event', event => { + unexpectedEvent = event; + }); + sander.writeFileSync('test/_tmp/input/dep.js', '= invalid'); + return wait(400).then(() => assert.strictEqual(unexpectedEvent, false)); + } + ]); + }); + }); + it('stops watching files that are no longer part of the graph', () => { return sander .copydir('test/watch/samples/dependency') @@ -888,12 +950,26 @@ describe('rollup.watch', () => { format: 'cjs' }, plugins: { - transform() { - this.addWatchFile('test/_tmp/input/watched'); - return `export default "${sander - .readFileSync('test/_tmp/input/watched') - .toString() - .trim()}"`; + resolveId(id) { + if (id === 'dep') { + return id; + } + }, + load(id) { + if (id === 'dep') { + return `throw new Error('This should not be executed);`; + } + }, + transform(code, id) { + if (id.endsWith('main.js')) { + return `export { value as default } from 'dep';`; + } else { + this.addWatchFile('test/_tmp/input/watched'); + return `export const value = "${sander + .readFileSync('test/_tmp/input/watched') + .toString() + .trim()}"`; + } } }, watch: { chokidar } From 4d6cd53916d97813c55ec51d23d221deb60cfec0 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 13:11:09 +0200 Subject: [PATCH 14/16] Seems like watching missing files just does not work out reliably on all systems --- src/watch/fileWatchers.ts | 8 ++---- test/watch/index.js | 52 +++++---------------------------------- 2 files changed, 8 insertions(+), 52 deletions(-) diff --git a/src/watch/fileWatchers.ts b/src/watch/fileWatchers.ts index 756cdd7262c..dbcee07443c 100644 --- a/src/watch/fileWatchers.ts +++ b/src/watch/fileWatchers.ts @@ -60,12 +60,8 @@ export default class FileWatcher { const handleWatchEvent = (event: string) => { if (event === 'rename' || event === 'unlink') { - if (chokidarOptions) { - modifiedTime = -1; - } else { - this.close(); - group.delete(id); - } + this.close(); + group.delete(id); this.trigger(id); return; } else { diff --git a/test/watch/index.js b/test/watch/index.js index 56f065858f5..a3a39584817 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -11,7 +11,7 @@ function wait(ms) { }); } -describe.only('rollup.watch', () => { +describe('rollup.watch', () => { beforeEach(() => { process.chdir(cwd); return sander.rimraf('test/_tmp'); @@ -64,9 +64,11 @@ describe.only('rollup.watch', () => { runTests(false); }); - describe('chokidar', () => { - runTests(true); - }); + if (!process.env.CI) { + describe('chokidar', () => { + runTests(true); + }); + } function runTests(chokidar) { it('watches a file', () => { @@ -354,48 +356,6 @@ describe.only('rollup.watch', () => { }); }); - if (chokidar) { - it('recovers from an error even when an entry file was deleted and recreated', () => { - return sander - .copydir('test/watch/samples/basic') - .to('test/_tmp/input') - .then(() => { - const watcher = rollup.watch({ - input: 'test/_tmp/input/main.js', - output: { - file: 'test/_tmp/output/bundle.js', - format: 'cjs' - }, - watch: { chokidar } - }); - - return sequence(watcher, [ - 'START', - 'BUNDLE_START', - 'BUNDLE_END', - 'END', - () => { - assert.strictEqual(run('../_tmp/output/bundle.js'), 42); - sander.unlinkSync('test/_tmp/input/main.js'); - }, - 'START', - 'BUNDLE_START', - 'ERROR', - () => { - sander.writeFileSync('test/_tmp/input/main.js', 'export default 43;'); - }, - 'START', - 'BUNDLE_START', - 'BUNDLE_END', - 'END', - () => { - assert.strictEqual(run('../_tmp/output/bundle.js'), 43); - } - ]); - }); - }); - } - it('handles closing the watcher during a build', () => { return sander .copydir('test/watch/samples/basic') From ec973d1fccda0cdd3a4f6c5146c72891b172f399 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 28 Aug 2019 13:23:41 +0200 Subject: [PATCH 15/16] Remove unused check --- src/Module.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index ab77c688548..50e31c3ee49 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -271,10 +271,7 @@ export default class Module { props.frame = getCodeFrame(this.originalCode, location.line, location.column); } - const watchFiles = Object.keys(this.graph.watchFiles); - if (watchFiles.length > 0) { - props.watchFiles = watchFiles; - } + props.watchFiles = Object.keys(this.graph.watchFiles); error(props); } From 64180e9b49a19ea8300aab1ec551e726df255068 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 29 Aug 2019 17:00:32 +0200 Subject: [PATCH 16/16] Add test for virtual files --- src/watch/fileWatchers.ts | 5 ++- test/watch/index.js | 49 ++++++++++++++++++++++++++++++ test/watch/samples/virtual/main.js | 1 + 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 test/watch/samples/virtual/main.js diff --git a/src/watch/fileWatchers.ts b/src/watch/fileWatchers.ts index dbcee07443c..c75bdc4e030 100644 --- a/src/watch/fileWatchers.ts +++ b/src/watch/fileWatchers.ts @@ -53,9 +53,8 @@ export default class FileWatcher { // can't watch files that don't exist (e.g. injected // by plugins somehow) return; - } else { - throw err; } + throw err; } const handleWatchEvent = (event: string) => { @@ -88,7 +87,7 @@ export default class FileWatcher { group.set(id, this); } - addTask(task: Task, isTransformDependency = false) { + addTask(task: Task, isTransformDependency: boolean) { if (isTransformDependency) this.transformDependencyTasks.add(task); else this.tasks.add(task); } diff --git a/test/watch/index.js b/test/watch/index.js index a3a39584817..4605b511151 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -105,6 +105,55 @@ describe('rollup.watch', () => { }); }); + it('does not fail for virtual files', () => { + return sander + .copydir('test/watch/samples/basic') + .to('test/_tmp/input') + .then(() => { + const watcher = rollup.watch({ + input: 'test/_tmp/input/main.js', + plugins: { + resolveId(id) { + if (id === 'virtual') { + return id; + } + }, + load(id) { + if (id === 'virtual') { + return `export const value = 42;`; + } + } + }, + output: { + file: 'test/_tmp/output/bundle.js', + format: 'cjs' + }, + watch: { chokidar } + }); + + return sequence(watcher, [ + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 42); + sander.writeFileSync( + 'test/_tmp/input/main.js', + "import {value} from 'virtual';\nexport default value + 1;" + ); + }, + 'START', + 'BUNDLE_START', + 'BUNDLE_END', + 'END', + () => { + assert.strictEqual(run('../_tmp/output/bundle.js'), 43); + } + ]); + }); + }); + it('passes file events to the watchChange plugin hook once for each change', () => { let watchChangeCnt = 0; return sander diff --git a/test/watch/samples/virtual/main.js b/test/watch/samples/virtual/main.js new file mode 100644 index 00000000000..3f9d7753101 --- /dev/null +++ b/test/watch/samples/virtual/main.js @@ -0,0 +1 @@ +export { value as default } from 'virtual';