From 18d1aede0affda09513343a0c18127446352b8f6 Mon Sep 17 00:00:00 2001 From: Michael Keller Date: Sun, 8 Sep 2019 03:36:07 -0400 Subject: [PATCH] Watch files onbuild, even if build fails (#3081) * Watch files onbuild, even if build fails * Add array, not graph, to error object * Update error format for tests * Update watch tests to throw ERRORs, not FATAL * Add test for recovering from error on watch * Fix eslint-utils security vulnerability * More restrictive object testing, ts fixes * Better type checking * Better type checking * 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 * 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 * Only keep watching deleted files when using chokidar * Improve coverage and remove unused edge cases * Seems like watching missing files just does not work out reliably on all systems * Remove unused check * Add test for virtual files --- src/Graph.ts | 3 +- src/Module.ts | 5 +- src/ModuleLoader.ts | 1 + src/rollup/types.d.ts | 5 +- src/watch/fileWatchers.ts | 6 +- src/watch/index.ts | 90 +++---- .../cannot-call-external-namespace/_config.js | 1 + .../cannot-call-internal-namespace/_config.js | 1 + .../samples/default-not-reexported/_config.js | 5 + .../samples/double-default-export/_config.js | 1 + .../samples/double-named-export/_config.js | 1 + .../samples/double-named-reexport/_config.js | 1 + .../samples/duplicate-import-fails/_config.js | 1 + .../_config.js | 1 + .../_config.js | 1 + .../samples/error-parse-json/_config.js | 1 + .../error-parse-unknown-extension/_config.js | 1 + .../export-not-at-top-level-fails/_config.js | 1 + .../import-not-at-top-level-fails/_config.js | 1 + .../import-of-unexported-fails/_config.js | 1 + .../_config.js | 3 +- .../_config.js | 1 + .../namespace-update-import-fails/_config.js | 1 + .../samples/reassign-import-fails/_config.js | 1 + .../_config.js | 1 + .../samples/reexport-missing-error/_config.js | 1 + .../_config.js | 1 + test/watch/index.js | 248 ++++++++++++++++-- test/watch/samples/dependency/dep.js | 1 + test/watch/samples/dependency/main.js | 2 + test/watch/samples/error/main.js | 2 + test/watch/samples/virtual/main.js | 1 + 32 files changed, 310 insertions(+), 81 deletions(-) create mode 100644 test/watch/samples/dependency/dep.js create mode 100644 test/watch/samples/dependency/main.js create mode 100644 test/watch/samples/error/main.js create mode 100644 test/watch/samples/virtual/main.js diff --git a/src/Graph.ts b/src/Graph.ts index b210a13ba92..6ad883f4110 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); } @@ -331,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 ea045ffffc4..50e31c3ee49 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -218,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) { @@ -244,7 +244,6 @@ export default class Module { error(props: RollupError, pos: number) { if (pos !== undefined) { props.pos = pos; - let location = locate(this.code, pos, { offsetLine: 1 }); try { location = getOriginalLocation(this.sourcemapChain, location); @@ -272,6 +271,8 @@ export default class Module { props.frame = getCodeFrame(this.originalCode, location.line, location.column); } + props.watchFiles = Object.keys(this.graph.watchFiles); + error(props); } 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/rollup/types.d.ts b/src/rollup/types.d.ts index f24e305b4d7..850e5c8136f 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -5,6 +5,7 @@ export const VERSION: string; export interface RollupError extends RollupLogProps { stack?: string; + watchFiles?: string[]; } export interface RollupWarning extends RollupLogProps { @@ -105,7 +106,7 @@ export interface TransformModuleJSON { originalSourcemap: ExistingDecodedSourceMap | null; resolvedIds?: ResolvedIdMap; sourcemapChain: DecodedSourceMapOrMissing[]; - transformDependencies: string[] | null; + transformDependencies: string[]; } export interface ModuleJSON extends TransformModuleJSON { @@ -527,7 +528,7 @@ export interface SerializablePluginCache { } export interface RollupCache { - modules?: ModuleJSON[]; + modules: ModuleJSON[]; plugins?: Record; } diff --git a/src/watch/fileWatchers.ts b/src/watch/fileWatchers.ts index 97c8084694b..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) => { @@ -63,6 +62,7 @@ export default class FileWatcher { this.close(); group.delete(id); this.trigger(id); + return; } else { let stats: fs.Stats; try { @@ -87,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/src/watch/index.ts b/src/watch/index.ts index 5bd0284ee8e..5f9a3e71be1 100644 --- a/src/watch/index.ts +++ b/src/watch/index.ts @@ -5,10 +5,10 @@ import createFilter from 'rollup-pluginutils/src/createFilter'; import rollup, { setWatcher } from '../rollup/index'; import { InputOptions, - ModuleJSON, OutputOptions, RollupBuild, RollupCache, + RollupError, RollupWatcher, WatcherOptions } from '../rollup/types'; @@ -25,7 +25,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) { @@ -48,9 +47,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 +71,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(); @@ -90,7 +91,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', { @@ -100,7 +100,7 @@ export class Watcher { .catch(error => { this.running = false; this.emit('event', { - code: this.succeeded ? 'ERROR' : 'FATAL', + code: 'ERROR', error }); }) @@ -114,7 +114,7 @@ export class Watcher { } export class Task { - cache: RollupCache; + cache: RollupCache = { modules: [] }; watchFiles: string[] = []; private chokidarOptions: WatchOptions; @@ -129,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; @@ -172,20 +171,19 @@ 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 => { - 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; - }); + } } this.watcher.invalidate(id); } @@ -211,27 +209,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) => { @@ -243,31 +221,39 @@ export class Task { result }); }) - .catch((error: Error) => { + .catch((error: RollupError) => { if (this.closed) return; - 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 => { + if (Array.isArray(error.watchFiles)) { + for (const id of error.watchFiles) { 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); + } + for (const module of this.cache.modules) { + 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/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/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')] } }; 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, diff --git a/test/watch/index.js b/test/watch/index.js index 1af42233e9e..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 @@ -281,6 +330,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') @@ -323,6 +405,110 @@ 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') + .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') @@ -402,12 +588,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); }); } ]); @@ -452,12 +643,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); }); } ]); @@ -763,12 +959,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 } @@ -1054,7 +1264,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.')); 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; 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; += 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';