From a9338d6af1595357a05b5a582290f69bc6d73d8b Mon Sep 17 00:00:00 2001 From: Christoph Pojer Date: Wed, 16 Nov 2016 20:11:48 -0800 Subject: [PATCH] New file watching implementation Summary: This is the next incremental step to rewrite node-haste. I apologize for the size of this diff but there is really no smaller way to do this. The current architecture passes a single file watcher instance into many classes that each subscribe to file changes. It's really hard to keep track of this. The new implementation reduces the listeners to two (will eventually be just one!) - one in DependencyGraph and one in it's parent's parent's parent (ugh! This doesn't make any sense). This should make it much more straightforward to understand what happens when a file changes. I was able to remove a bunch of tests because jest's watcher takes care of things like ignore patterns. Some of the tests were specifically testing for whether the change events were invoked and they are now much more straightforward as well by manually invoking the `processFileChange` methods. (Relanding a fixed version of D4161662) Reviewed By: kentaromiura Differential Revision: D4194378 fbshipit-source-id: 8c008247a911573f6b5f6b0b374d50d38f62a4f5 --- docs/GettingStarted.md | 4 +- local-cli/bundle/buildBundle.js | 2 +- local-cli/server/runServer.js | 18 +- packager/package.json | 27 --- packager/react-packager/index.js | 20 +- .../AssetServer/__tests__/AssetServer-test.js | 29 +-- .../react-packager/src/AssetServer/index.js | 18 +- packager/react-packager/src/Bundler/index.js | 22 +- packager/react-packager/src/Resolver/index.js | 11 +- .../src/Server/__tests__/Server-test.js | 65 ++---- packager/react-packager/src/Server/index.js | 73 ++---- .../DependencyGraph/DeprecatedAssetMap.js | 4 +- .../node-haste/FileWatcher/__mocks__/sane.js | 14 -- .../FileWatcher/__tests__/FileWatcher-test.js | 75 ------- .../src/node-haste/FileWatcher/index.js | 123 ---------- .../src/node-haste/ModuleCache.js | 20 +- .../__tests__/DependencyGraph-test.js | 212 +----------------- .../src/node-haste/__tests__/Module-test.js | 67 +++--- .../react-packager/src/node-haste/fastfs.js | 37 +-- .../react-packager/src/node-haste/index.js | 91 ++++---- 20 files changed, 191 insertions(+), 741 deletions(-) delete mode 100644 packager/react-packager/src/node-haste/FileWatcher/__mocks__/sane.js delete mode 100644 packager/react-packager/src/node-haste/FileWatcher/__tests__/FileWatcher-test.js delete mode 100644 packager/react-packager/src/node-haste/FileWatcher/index.js diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index 38ba3b785ac111..04cbaf4062efaa 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -362,8 +362,6 @@ If everything is set up correctly, you should see your new app running in your A -> If you hit a `ERROR Watcher took too long to load`, try increasing the timeout in [this file](https://github.com/facebook/react-native/blob/5fa33f3d07f8595a188f6fe04d6168a6ede1e721/packager/react-packager/src/DependencyResolver/FileWatcher/index.js#L16) (under your `node_modules/react-native/`). - > If you're targeting API level 23, the app might crash on first launch with an error smilar to `Unable to add window android.view.ViewRootImpl$W@c51fa6 -- permission denied for this window type`. To fix this, you need to go to `System settings > Apps > Configure apps > Draw over other apps` and grant the permission for the app. NOTE: Many React Native modules haven't been tested on Marshmallow and might break. Please throughly test the app if you target API level 23 and file a bug report if you find that something is broken. @@ -467,7 +465,7 @@ if (window.location.hash !== '' && window.location.hash !== 'content') { // cont } // We would have broken out if both targetPlatform and devOS hadn't been filled. display('os', devOS); - display('platform', targetPlatform); + display('platform', targetPlatform); foundHash = true; break; } diff --git a/local-cli/bundle/buildBundle.js b/local-cli/bundle/buildBundle.js index 047ec2ac504a9a..d950ff802084d2 100644 --- a/local-cli/bundle/buildBundle.js +++ b/local-cli/bundle/buildBundle.js @@ -55,8 +55,8 @@ function buildBundle(args, config, output = outputBundle, packagerInstance) { getTransformOptionsModulePath: config.getTransformOptionsModulePath, transformModulePath: transformModulePath, extraNodeModules: config.extraNodeModules, - nonPersistent: true, resetCache: args.resetCache, + watch: false, }; packagerInstance = new Server(options); diff --git a/local-cli/server/runServer.js b/local-cli/server/runServer.js index 5d2ae1730c0593..46526abdf9bde9 100644 --- a/local-cli/server/runServer.js +++ b/local-cli/server/runServer.js @@ -8,12 +8,14 @@ */ 'use strict'; +const InspectorProxy = require('./util/inspectorProxy.js'); const ReactPackager = require('../../packager/react-packager'); const attachHMRServer = require('./util/attachHMRServer'); const connect = require('connect'); const copyToClipBoardMiddleware = require('./middleware/copyToClipBoardMiddleware'); const cpuProfilerMiddleware = require('./middleware/cpuProfilerMiddleware'); +const defaultAssetExts = require('../../packager/defaults').assetExts; const getDevToolsMiddleware = require('./middleware/getDevToolsMiddleware'); const heapCaptureMiddleware = require('./middleware/heapCaptureMiddleware.js'); const http = require('http'); @@ -25,10 +27,8 @@ const openStackFrameInEditorMiddleware = require('./middleware/openStackFrameInE const path = require('path'); const statusPageMiddleware = require('./middleware/statusPageMiddleware.js'); const systraceProfileMiddleware = require('./middleware/systraceProfileMiddleware.js'); -const webSocketProxy = require('./util/webSocketProxy.js'); -const InspectorProxy = require('./util/inspectorProxy.js'); -const defaultAssetExts = require('../../packager/defaults').assetExts; const unless = require('./middleware/unless'); +const webSocketProxy = require('./util/webSocketProxy.js'); function runServer(args, config, readyCallback) { var wsProxy = null; @@ -86,17 +86,17 @@ function getPackagerServer(args, config) { undefined; return ReactPackager.createServer({ - nonPersistent: args.nonPersistent, - projectRoots: args.projectRoots, + assetExts: defaultAssetExts.concat(args.assetExts), + assetRoots: args.assetRoots, blacklistRE: config.getBlacklistRE(), cacheVersion: '3', - getTransformOptionsModulePath: config.getTransformOptionsModulePath, - transformModulePath: transformModulePath, extraNodeModules: config.extraNodeModules, - assetRoots: args.assetRoots, - assetExts: defaultAssetExts.concat(args.assetExts), + getTransformOptionsModulePath: config.getTransformOptionsModulePath, + projectRoots: args.projectRoots, resetCache: args.resetCache, + transformModulePath: transformModulePath, verbose: args.verbose, + watch: !args.nonPersistent, }); } diff --git a/packager/package.json b/packager/package.json index 700d9715c7aa22..126696df97bf1e 100644 --- a/packager/package.json +++ b/packager/package.json @@ -5,32 +5,5 @@ "repository": { "type": "git", "url": "git@github.com:facebook/react-native.git" - }, - "engines": { - "node": ">=4" - }, - "jest": { - "setupEnvScriptFile": "jest/setup.js", - "testPathIgnorePatterns": [ - "/node_modules/" - ], - "testFileExtensions": [ - "js" - ], - "unmockedModulePathPatterns": [ - "source-map" - ] - }, - "scripts": { - "test": "jest", - "lint": "node linter.js Examples/", - "start": "./packager.sh" - }, - "dependencies": { - "wordwrap": "^1.0.0" - }, - "devDependencies": { - "jest-cli": "git://github.com/facebook/jest#0.5.x", - "eslint": "0.9.2" } } diff --git a/packager/react-packager/index.js b/packager/react-packager/index.js index e8f31d07a04e29..2df2fff0bac09d 100644 --- a/packager/react-packager/index.js +++ b/packager/react-packager/index.js @@ -45,26 +45,14 @@ function createServer(options) { enableDebug(); } + options = Object.assign({}, options); + delete options.verbose; var Server = require('./src/Server'); - return new Server(omit(options, ['verbose'])); + return new Server(options); } function createNonPersistentServer(options) { Logger.disablePrinting(); - // Don't start the filewatcher or the cache. - if (options.nonPersistent == null) { - options.nonPersistent = true; - } - + options.watch = !options.nonPersistent; return createServer(options); } - -function omit(obj, blacklistedKeys) { - return Object.keys(obj).reduce((clone, key) => { - if (blacklistedKeys.indexOf(key) === -1) { - clone[key] = obj[key]; - } - - return clone; - }, {}); -} diff --git a/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js b/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js index 7e70d6bc4b8bf6..69b442bf7d6d50 100644 --- a/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js +++ b/packager/react-packager/src/AssetServer/__tests__/AssetServer-test.js @@ -15,17 +15,15 @@ jest.mock('fs'); const AssetServer = require('../'); const crypto = require('crypto'); -const {EventEmitter} = require('events'); const fs = require('fs'); const {objectContaining} = jasmine; describe('AssetServer', () => { - let fileWatcher; beforeEach(() => { const NodeHaste = require('../../node-haste'); - NodeHaste.getAssetDataFromName = require.requireActual('../../node-haste/lib/getAssetDataFromName'); - fileWatcher = new EventEmitter(); + NodeHaste.getAssetDataFromName = + require.requireActual('../../node-haste/lib/getAssetDataFromName'); }); describe('assetServer.get', () => { @@ -33,7 +31,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -59,7 +56,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -97,7 +93,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png', 'jpg'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -124,7 +119,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -147,7 +141,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -179,7 +172,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root', '/root2'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -208,7 +200,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -241,7 +232,6 @@ describe('AssetServer', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png', 'jpeg'], - fileWatcher, }); fs.__setMockFilesystem({ @@ -271,15 +261,14 @@ describe('AssetServer', () => { }); describe('hash:', () => { - let server, fileSystem; + let server, mockFS; beforeEach(() => { server = new AssetServer({ projectRoots: ['/root'], assetExts: ['jpg'], - fileWatcher, }); - fileSystem = { + mockFS = { 'root': { imgs: { 'b@1x.jpg': 'b1 image', @@ -290,13 +279,13 @@ describe('AssetServer', () => { } }; - fs.__setMockFilesystem(fileSystem); + fs.__setMockFilesystem(mockFS); }); it('uses the file contents to build the hash', () => { const hash = crypto.createHash('md5'); - for (const name in fileSystem.root.imgs) { - hash.update(fileSystem.root.imgs[name]); + for (const name in mockFS.root.imgs) { + hash.update(mockFS.root.imgs[name]); } return server.getAssetData('imgs/b.jpg').then(data => @@ -306,8 +295,8 @@ describe('AssetServer', () => { it('changes the hash when the passed-in file watcher emits an `all` event', () => { return server.getAssetData('imgs/b.jpg').then(initialData => { - fileSystem.root.imgs['b@4x.jpg'] = 'updated data'; - fileWatcher.emit('all', 'arbitrary', '/root', 'imgs/b@4x.jpg'); + mockFS.root.imgs['b@4x.jpg'] = 'updated data'; + server.onFileChange('all', '/root/imgs/b@4x.jpg'); return server.getAssetData('imgs/b.jpg').then(data => expect(data.hash).not.toEqual(initialData.hash) ); diff --git a/packager/react-packager/src/AssetServer/index.js b/packager/react-packager/src/AssetServer/index.js index e1a02f65484cee..a267eea6bd97c4 100644 --- a/packager/react-packager/src/AssetServer/index.js +++ b/packager/react-packager/src/AssetServer/index.js @@ -42,10 +42,6 @@ const validateOpts = declareOpts({ type: 'array', required: true, }, - fileWatcher: { - type: 'object', - required: true, - } }); class AssetServer { @@ -55,9 +51,6 @@ class AssetServer { this._assetExts = opts.assetExts; this._hashes = new Map(); this._files = new Map(); - - opts.fileWatcher - .on('all', (type, root, file) => this._onFileChange(type, root, file)); } get(assetPath, platform = null) { @@ -84,7 +77,6 @@ class AssetServer { data.scales = record.scales; data.files = record.files; - if (this._hashes.has(assetPath)) { data.hash = this._hashes.get(assetPath); return data; @@ -106,9 +98,8 @@ class AssetServer { }); } - _onFileChange(type, root, file) { - const asset = this._files.get(path.join(root, file)); - this._hashes.delete(asset); + onFileChange(type, filePath) { + this._hashes.delete(this._files.get(filePath)); } /** @@ -187,7 +178,10 @@ class AssetServer { } const rootsString = roots.map(s => `'${s}'`).join(', '); - throw new Error(`'${debugInfoFile}' could not be found, because '${dir}' is not a subdirectory of any of the roots (${rootsString})`); + throw new Error( + `'${debugInfoFile}' could not be found, because '${dir}' is not a ` + + `subdirectory of any of the roots (${rootsString})`, + ); }); } diff --git a/packager/react-packager/src/Bundler/index.js b/packager/react-packager/src/Bundler/index.js index 4165edcd9ea7bc..26e9b6646d52b1 100644 --- a/packager/react-packager/src/Bundler/index.js +++ b/packager/react-packager/src/Bundler/index.js @@ -79,10 +79,6 @@ const validateOpts = declareOpts({ type: 'object', required: false, }, - nonPersistent: { - type: 'boolean', - default: false, - }, assetRoots: { type: 'array', required: false, @@ -91,9 +87,9 @@ const validateOpts = declareOpts({ type: 'array', default: ['png'], }, - fileWatcher: { - type: 'object', - required: true, + watch: { + type: 'boolean', + default: false, }, assetServer: { type: 'object', @@ -124,10 +120,9 @@ type Options = { resetCache: boolean, transformModulePath: string, extraNodeModules: {}, - nonPersistent: boolean, assetRoots: Array, assetExts: Array, - fileWatcher: {}, + watch: boolean, assetServer: AssetServer, transformTimeoutInterval: ?number, allowBundleUpdates: boolean, @@ -191,7 +186,7 @@ class Bundler { blacklistRE: opts.blacklistRE, cache: this._cache, extraNodeModules: opts.extraNodeModules, - fileWatcher: opts.fileWatcher, + watch: opts.watch, minifyCode: this._transformer.minify, moduleFormat: opts.moduleFormat, polyfillModuleNames: opts.polyfillModuleNames, @@ -217,9 +212,12 @@ class Bundler { } } - kill() { + end() { this._transformer.kill(); - return this._cache.end(); + return Promise.all([ + this._cache.end(), + this.getResolver().getDependencyGraph().getWatcher().end(), + ]); } bundle(options: { diff --git a/packager/react-packager/src/Resolver/index.js b/packager/react-packager/src/Resolver/index.js index 7588d2384b3121..47c28123e191a3 100644 --- a/packager/react-packager/src/Resolver/index.js +++ b/packager/react-packager/src/Resolver/index.js @@ -35,9 +35,9 @@ const validateOpts = declareOpts({ type: 'array', default: [], }, - fileWatcher: { - type: 'object', - required: true, + watch: { + type: 'boolean', + default: false, }, assetExts: { type: 'array', @@ -101,14 +101,13 @@ class Resolver { providesModuleNodeModules: defaults.providesModuleNodeModules, platforms: defaults.platforms, preferNativePlatform: true, - fileWatcher: opts.fileWatcher, + watch: opts.watch, cache: opts.cache, shouldThrowOnUnresolvedErrors: (_, platform) => platform !== 'android', transformCode: opts.transformCode, transformCacheKey: opts.transformCacheKey, extraNodeModules: opts.extraNodeModules, assetDependencies: ['react-native/Libraries/Image/AssetRegistry'], - // for jest-haste-map resetCache: options.resetCache, moduleOptions: { cacheTransformResults: true, @@ -260,7 +259,7 @@ class Resolver { return this._minifyCode(path, code, map); } - getDependecyGraph() { + getDependencyGraph() { return this._depGraph; } } diff --git a/packager/react-packager/src/Server/__tests__/Server-test.js b/packager/react-packager/src/Server/__tests__/Server-test.js index d7dfc2aa2b7327..44f0cddf2c30ac 100644 --- a/packager/react-packager/src/Server/__tests__/Server-test.js +++ b/packager/react-packager/src/Server/__tests__/Server-test.js @@ -21,8 +21,6 @@ jest.setMock('worker-farm', function() { return () => {}; }) .mock('../../node-haste') .mock('../../Logger'); -let FileWatcher; - describe('processRequest', () => { let SourceMapConsumer, Bundler, Server, AssetServer, Promise; beforeEach(() => { @@ -62,12 +60,9 @@ describe('processRequest', () => { ); const invalidatorFunc = jest.fn(); - const watcherFunc = jest.fn(); let requestHandler; - let triggerFileChange; beforeEach(() => { - FileWatcher = require('../../node-haste').FileWatcher; Bundler.prototype.bundle = jest.fn(() => Promise.resolve({ getSource: () => 'this is the source', @@ -75,19 +70,10 @@ describe('processRequest', () => { getEtag: () => 'this is an etag', })); - FileWatcher.prototype.on = function(eventType, callback) { - if (eventType !== 'all') { - throw new Error('Can only handle "all" event in watcher.'); - } - watcherFunc.apply(this, arguments); - triggerFileChange = callback; - return this; - }; - Bundler.prototype.invalidateFile = invalidatorFunc; Bundler.prototype.getResolver = jest.fn().mockReturnValue({ - getDependecyGraph: jest.fn().mockReturnValue({ + getDependencyGraph: jest.fn().mockReturnValue({ getHasteMap: jest.fn().mockReturnValue({on: jest.fn()}), load: jest.fn(() => Promise.resolve()), }), @@ -97,7 +83,7 @@ describe('processRequest', () => { requestHandler = server.processRequest.bind(server); }); - pit('returns JS bundle source on request of *.bundle', () => { + it('returns JS bundle source on request of *.bundle', () => { return makeRequest( requestHandler, 'mybundle.bundle?runModule=true', @@ -107,7 +93,7 @@ describe('processRequest', () => { ); }); - pit('returns JS bundle source on request of *.bundle (compat)', () => { + it('returns JS bundle source on request of *.bundle (compat)', () => { return makeRequest( requestHandler, 'mybundle.runModule.bundle' @@ -116,7 +102,7 @@ describe('processRequest', () => { ); }); - pit('returns ETag header on request of *.bundle', () => { + it('returns ETag header on request of *.bundle', () => { return makeRequest( requestHandler, 'mybundle.bundle?runModule=true' @@ -125,7 +111,7 @@ describe('processRequest', () => { }); }); - pit('returns 304 on request of *.bundle when if-none-match equals the ETag', () => { + it('returns 304 on request of *.bundle when if-none-match equals the ETag', () => { return makeRequest( requestHandler, 'mybundle.bundle?runModule=true', @@ -135,7 +121,7 @@ describe('processRequest', () => { }); }); - pit('returns sourcemap on request of *.map', () => { + it('returns sourcemap on request of *.map', () => { return makeRequest( requestHandler, 'mybundle.map?runModule=true' @@ -144,7 +130,7 @@ describe('processRequest', () => { ); }); - pit('works with .ios.js extension', () => { + it('works with .ios.js extension', () => { return makeRequest( requestHandler, 'index.ios.includeRequire.bundle' @@ -169,7 +155,7 @@ describe('processRequest', () => { }); }); - pit('passes in the platform param', function() { + it('passes in the platform param', function() { return makeRequest( requestHandler, 'index.bundle?platform=ios' @@ -194,7 +180,7 @@ describe('processRequest', () => { }); }); - pit('passes in the assetPlugin param', function() { + it('passes in the assetPlugin param', function() { return makeRequest( requestHandler, 'index.bundle?assetPlugin=assetPlugin1&assetPlugin=assetPlugin2' @@ -219,24 +205,13 @@ describe('processRequest', () => { }); }); - pit('watches all files in projectRoot', () => { - return makeRequest( - requestHandler, - 'mybundle.bundle?runModule=true' - ).then(() => { - expect(watcherFunc.mock.calls[0][0]).toEqual('all'); - expect(watcherFunc.mock.calls[0][1]).not.toBe(null); - }); - }); - describe('file changes', () => { - pit('invalides files in bundle when file is updated', () => { + it('invalides files in bundle when file is updated', () => { return makeRequest( requestHandler, 'mybundle.bundle?runModule=true' ).then(() => { - const onFileChange = watcherFunc.mock.calls[0][1]; - onFileChange('all','path/file.js', options.projectRoots[0]); + server.onFileChange('all', options.projectRoots[0] + '/path/file.js'); expect(invalidatorFunc.mock.calls[0][0]).toEqual('root/path/file.js'); }); }); @@ -273,7 +248,7 @@ describe('processRequest', () => { jest.runAllTicks(); - triggerFileChange('all','path/file.js', options.projectRoots[0]); + server.onFileChange('all', options.projectRoots[0] + 'path/file.js'); jest.runAllTimers(); jest.runAllTicks(); @@ -322,7 +297,7 @@ describe('processRequest', () => { jest.runAllTicks(); - triggerFileChange('all','path/file.js', options.projectRoots[0]); + server.onFileChange('all', options.projectRoots[0] + 'path/file.js'); jest.runAllTimers(); jest.runAllTicks(); @@ -355,7 +330,7 @@ describe('processRequest', () => { it('should hold on to request and inform on change', () => { server.processRequest(req, res); - triggerFileChange('all', 'path/file.js', options.projectRoots[0]); + server.onFileChange('all', options.projectRoots[0] + 'path/file.js'); jest.runAllTimers(); expect(res.end).toBeCalledWith(JSON.stringify({changed: true})); }); @@ -364,7 +339,7 @@ describe('processRequest', () => { server.processRequest(req, res); req.emit('close'); jest.runAllTimers(); - triggerFileChange('all', 'path/file.js', options.projectRoots[0]); + server.onFileChange('all', options.projectRoots[0] + 'path/file.js'); jest.runAllTimers(); expect(res.end).not.toBeCalled(); }); @@ -428,7 +403,7 @@ describe('processRequest', () => { }); describe('buildbundle(options)', () => { - pit('Calls the bundler with the correct args', () => { + it('Calls the bundler with the correct args', () => { return server.buildBundle({ entryFile: 'foo file' }).then(() => @@ -451,7 +426,7 @@ describe('processRequest', () => { }); describe('buildBundleFromUrl(options)', () => { - pit('Calls the bundler with the correct args', () => { + it('Calls the bundler with the correct args', () => { return server.buildBundleFromUrl('/path/to/foo.bundle?dev=false&runModule=false') .then(() => expect(Bundler.prototype.bundle).toBeCalledWith({ @@ -474,7 +449,7 @@ describe('processRequest', () => { }); describe('/symbolicate endpoint', () => { - pit('should symbolicate given stack trace', () => { + it('should symbolicate given stack trace', () => { const body = JSON.stringify({stack: [{ file: 'http://foo.bundle?platform=ios', lineNumber: 2100, @@ -508,7 +483,7 @@ describe('processRequest', () => { }); }); - pit('ignores `/debuggerWorker.js` stack frames', () => { + it('ignores `/debuggerWorker.js` stack frames', () => { const body = JSON.stringify({stack: [{ file: 'http://localhost:8081/debuggerWorker.js', lineNumber: 123, @@ -532,7 +507,7 @@ describe('processRequest', () => { }); describe('/symbolicate handles errors', () => { - pit('should symbolicate given stack trace', () => { + it('should symbolicate given stack trace', () => { const body = 'clearly-not-json'; console.error = jest.fn(); diff --git a/packager/react-packager/src/Server/index.js b/packager/react-packager/src/Server/index.js index cf63c47e3d4e61..7fdcffa92122de 100644 --- a/packager/react-packager/src/Server/index.js +++ b/packager/react-packager/src/Server/index.js @@ -9,7 +9,6 @@ 'use strict'; const AssetServer = require('../AssetServer'); -const FileWatcher = require('../node-haste').FileWatcher; const getPlatformExtension = require('../node-haste').getPlatformExtension; const Bundler = require('../Bundler'); const MultipartResponse = require('./MultipartResponse'); @@ -76,7 +75,7 @@ const validateOpts = declareOpts({ type: 'object', required: false, }, - nonPersistent: { + watch: { type: 'boolean', default: false, }, @@ -200,57 +199,32 @@ const NODE_MODULES = `${path.sep}node_modules${path.sep}`; class Server { constructor(options) { const opts = this._opts = validateOpts(options); + const processFileChange = + ({type, filePath, stat}) => this.onFileChange(type, filePath, stat); this._projectRoots = opts.projectRoots; this._bundles = Object.create(null); this._changeWatchers = []; this._fileChangeListeners = []; - const assetGlobs = opts.assetExts.map(ext => '**/*.' + ext); - - let watchRootConfigs = opts.projectRoots.map(dir => { - return { - dir: dir, - globs: [ - '**/*.js', - '**/*.json', - ].concat(assetGlobs), - }; - }); - - if (opts.assetRoots != null) { - watchRootConfigs = watchRootConfigs.concat( - opts.assetRoots.map(dir => { - return { - dir: dir, - globs: assetGlobs, - }; - }) - ); - } - - this._fileWatcher = options.nonPersistent - ? FileWatcher.createDummyWatcher() - : new FileWatcher(watchRootConfigs, {useWatchman: true}); - this._assetServer = new AssetServer({ assetExts: opts.assetExts, - fileWatcher: this._fileWatcher, projectRoots: opts.projectRoots, }); const bundlerOpts = Object.create(opts); - bundlerOpts.fileWatcher = this._fileWatcher; bundlerOpts.assetServer = this._assetServer; - bundlerOpts.allowBundleUpdates = !options.nonPersistent; + bundlerOpts.allowBundleUpdates = options.watch; + bundlerOpts.watch = options.watch; this._bundler = new Bundler(bundlerOpts); - this._fileWatcher.on('all', this._onFileChange.bind(this)); - // changes to the haste map can affect resolution of files in the bundle - const dependencyGraph = this._bundler.getResolver().getDependecyGraph(); - + const dependencyGraph = this._bundler.getResolver().getDependencyGraph(); dependencyGraph.load().then(() => { + dependencyGraph.getWatcher().on( + 'change', + ({eventsQueue}) => eventsQueue.forEach(processFileChange), + ); dependencyGraph.getHasteMap().on('change', () => { debug('Clearing bundle cache due to haste map change'); this._clearBundles(); @@ -282,10 +256,7 @@ class Server { } end() { - Promise.all([ - this._fileWatcher.end(), - this._bundler.kill(), - ]); + return this._bundler.end(); } setHMRFileChangeListener(listener) { @@ -299,7 +270,7 @@ class Server { } buildBundle(options) { - return this._bundler.getResolver().getDependecyGraph().load().then(() => { + return this._bundler.getResolver().getDependencyGraph().load().then(() => { if (!options.platform) { options.platform = getPlatformExtension(options.entryFile); } @@ -370,9 +341,9 @@ class Server { }); } - _onFileChange(type, filepath, root) { - const absPath = path.join(root, filepath); - this._bundler.invalidateFile(absPath); + onFileChange(type, filePath, stat) { + this._assetServer.onFileChange(type, filePath, stat); + this._bundler.invalidateFile(filePath); // If Hot Loading is enabled avoid rebuilding bundles and sending live // updates. Instead, send the HMR updates right away and clear the bundles @@ -380,26 +351,26 @@ class Server { if (this._hmrFileChangeListener) { // Clear cached bundles in case user reloads this._clearBundles(); - this._hmrFileChangeListener(absPath, this._bundler.stat(absPath)); + this._hmrFileChangeListener(filePath, this._bundler.stat(filePath)); return; - } else if (type !== 'change' && absPath.indexOf(NODE_MODULES) !== -1) { + } else if (type !== 'change' && filePath.indexOf(NODE_MODULES) !== -1) { // node module resolution can be affected by added or removed files debug('Clearing bundles due to potential node_modules resolution change'); this._clearBundles(); } Promise.all( - this._fileChangeListeners.map(listener => listener(absPath)) + this._fileChangeListeners.map(listener => listener(filePath)) ).then( - () => this._onFileChangeComplete(absPath), - () => this._onFileChangeComplete(absPath) + () => this._onFileChangeComplete(filePath), + () => this._onFileChangeComplete(filePath) ); } - _onFileChangeComplete(absPath) { + _onFileChangeComplete(filePath) { // Make sure the file watcher event runs through the system before // we rebuild the bundles. - this._debouncedFileChangeHandler(absPath); + this._debouncedFileChangeHandler(filePath); } _clearBundles() { diff --git a/packager/react-packager/src/node-haste/DependencyGraph/DeprecatedAssetMap.js b/packager/react-packager/src/node-haste/DependencyGraph/DeprecatedAssetMap.js index fc24c994fa1116..e72829a0c55628 100644 --- a/packager/react-packager/src/node-haste/DependencyGraph/DeprecatedAssetMap.js +++ b/packager/react-packager/src/node-haste/DependencyGraph/DeprecatedAssetMap.js @@ -54,14 +54,14 @@ class DeprecatedAssetMap { } } - processFileChange(type, filePath, root, fstat) { + processFileChange(type, filePath, fstat) { const name = assetName(filePath); if (type === 'change' || type === 'delete') { delete this._map[name]; } if (type === 'change' || type === 'add') { - this._processAsset(path.join(root, filePath)); + this._processAsset(filePath); } } } diff --git a/packager/react-packager/src/node-haste/FileWatcher/__mocks__/sane.js b/packager/react-packager/src/node-haste/FileWatcher/__mocks__/sane.js deleted file mode 100644 index f59fa9104eb271..00000000000000 --- a/packager/react-packager/src/node-haste/FileWatcher/__mocks__/sane.js +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ -'use strict'; - -module.exports = { - WatchmanWatcher: jest.genMockFromModule('sane/src/watchman_watcher'), - NodeWatcher: jest.genMockFromModule('sane/src/node_watcher'), -}; diff --git a/packager/react-packager/src/node-haste/FileWatcher/__tests__/FileWatcher-test.js b/packager/react-packager/src/node-haste/FileWatcher/__tests__/FileWatcher-test.js deleted file mode 100644 index 03f2ad03611b4a..00000000000000 --- a/packager/react-packager/src/node-haste/FileWatcher/__tests__/FileWatcher-test.js +++ /dev/null @@ -1,75 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ -'use strict'; - -jest - .dontMock('util') - .dontMock('events') - .dontMock('../') - .setMock('child_process', { - execSync: () => '/usr/bin/watchman', - }); - -describe('FileWatcher', () => { - let WatchmanWatcher; - let FileWatcher; - let config; - - beforeEach(() => { - jest.resetModules(); - const sane = require('sane'); - WatchmanWatcher = sane.WatchmanWatcher; - WatchmanWatcher.prototype.once.mockImplementation( - (type, callback) => callback() - ); - - FileWatcher = require('../'); - - config = [{ - dir: 'rootDir', - globs: [ - '**/*.js', - '**/*.json', - ], - }]; - }); - - pit('gets the watcher instance when ready', () => { - const fileWatcher = new FileWatcher(config); - return fileWatcher.getWatchers().then(watchers => { - watchers.forEach(watcher => { - expect(watcher instanceof WatchmanWatcher).toBe(true); - }); - }); - }); - - pit('emits events', () => { - let cb; - WatchmanWatcher.prototype.on.mockImplementation((type, callback) => { - cb = callback; - }); - const fileWatcher = new FileWatcher(config); - const handler = jest.genMockFn(); - fileWatcher.on('all', handler); - return fileWatcher.getWatchers().then(watchers => { - cb(1, 2, 3, 4); - jest.runAllTimers(); - expect(handler.mock.calls[0]).toEqual([1, 2, 3, 4]); - }); - }); - - pit('ends the watcher', () => { - const fileWatcher = new FileWatcher(config); - WatchmanWatcher.prototype.close.mockImplementation(callback => callback()); - - return fileWatcher.end().then(() => { - expect(WatchmanWatcher.prototype.close).toBeCalled(); - }); - }); -}); diff --git a/packager/react-packager/src/node-haste/FileWatcher/index.js b/packager/react-packager/src/node-haste/FileWatcher/index.js deleted file mode 100644 index 782aed442a9146..00000000000000 --- a/packager/react-packager/src/node-haste/FileWatcher/index.js +++ /dev/null @@ -1,123 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ -'use strict'; - -const EventEmitter = require('events').EventEmitter; -const denodeify = require('denodeify'); -const sane = require('sane'); -const execSync = require('child_process').execSync; - -const MAX_WAIT_TIME = 360000; - -const detectWatcherClass = () => { - try { - execSync('watchman version', {stdio: 'ignore'}); - return sane.WatchmanWatcher; - } catch (e) {} - return sane.NodeWatcher; -}; - -const WatcherClass = detectWatcherClass(); - -let inited = false; - -class FileWatcher extends EventEmitter { - - constructor(rootConfigs) { - if (inited) { - throw new Error('FileWatcher can only be instantiated once'); - } - inited = true; - - super(); - this._watcherByRoot = Object.create(null); - - const watcherPromises = rootConfigs.map((rootConfig) => { - return this._createWatcher(rootConfig); - }); - - this._loading = Promise.all(watcherPromises).then(watchers => { - watchers.forEach((watcher, i) => { - this._watcherByRoot[rootConfigs[i].dir] = watcher; - watcher.on( - 'all', - // args = (type, filePath, root, stat) - (...args) => this.emit('all', ...args) - ); - }); - return watchers; - }); - } - - getWatchers() { - return this._loading; - } - - getWatcherForRoot(root) { - return this._loading.then(() => this._watcherByRoot[root]); - } - - isWatchman() { - return Promise.resolve(FileWatcher.canUseWatchman()); - } - - end() { - inited = false; - return this._loading.then( - (watchers) => watchers.map( - watcher => denodeify(watcher.close).call(watcher) - ) - ); - } - - _createWatcher(rootConfig) { - const watcher = new WatcherClass(rootConfig.dir, { - glob: rootConfig.globs, - dot: false, - }); - - return new Promise((resolve, reject) => { - const rejectTimeout = setTimeout( - () => reject(new Error(timeoutMessage(WatcherClass))), - MAX_WAIT_TIME - ); - - watcher.once('ready', () => { - clearTimeout(rejectTimeout); - resolve(watcher); - }); - }); - } - - static createDummyWatcher() { - return Object.assign(new EventEmitter(), { - isWatchman: () => Promise.resolve(false), - end: () => Promise.resolve(), - }); - } - - static canUseWatchman() { - return WatcherClass == sane.WatchmanWatcher; - } -} - -function timeoutMessage(Watcher) { - const lines = [ - 'Watcher took too long to load (' + Watcher.name + ')', - ]; - if (Watcher === sane.WatchmanWatcher) { - lines.push( - 'Try running `watchman version` from your terminal', - 'https://facebook.github.io/watchman/docs/troubleshooting.html', - ); - } - return lines.join('\n'); -} - -module.exports = FileWatcher; diff --git a/packager/react-packager/src/node-haste/ModuleCache.js b/packager/react-packager/src/node-haste/ModuleCache.js index 655b01d15c0320..a963c24af6b54a 100644 --- a/packager/react-packager/src/node-haste/ModuleCache.js +++ b/packager/react-packager/src/node-haste/ModuleCache.js @@ -16,8 +16,6 @@ const Module = require('./Module'); const Package = require('./Package'); const Polyfill = require('./Polyfill'); -const path = require('path'); - import type Cache from './Cache'; import type { DepGraphHelpers, @@ -69,8 +67,6 @@ class ModuleCache { this._assetDependencies = assetDependencies; this._moduleOptions = moduleOptions; this._packageModuleMap = new WeakMap(); - - fastfs.on('change', this._processFileChange.bind(this)); } getModule(filePath: string) { @@ -149,16 +145,14 @@ class ModuleCache { }); } - _processFileChange(type, filePath, root) { - const absPath = path.join(root, filePath); - - if (this._moduleCache[absPath]) { - this._moduleCache[absPath].invalidate(); - delete this._moduleCache[absPath]; + processFileChange(type: string, filePath: string) { + if (this._moduleCache[filePath]) { + this._moduleCache[filePath].invalidate(); + delete this._moduleCache[filePath]; } - if (this._packageCache[absPath]) { - this._packageCache[absPath].invalidate(); - delete this._packageCache[absPath]; + if (this._packageCache[filePath]) { + this._packageCache[filePath].invalidate(); + delete this._packageCache[filePath]; } } } diff --git a/packager/react-packager/src/node-haste/__tests__/DependencyGraph-test.js b/packager/react-packager/src/node-haste/__tests__/DependencyGraph-test.js index 8e58f387519a88..be472c9a837c32 100644 --- a/packager/react-packager/src/node-haste/__tests__/DependencyGraph-test.js +++ b/packager/react-packager/src/node-haste/__tests__/DependencyGraph-test.js @@ -61,12 +61,6 @@ describe('DependencyGraph', function() { jest.resetModules(); Module = require('../Module'); - const fileWatcher = { - on: function() { - return this; - }, - isWatchman: () => Promise.resolve(false), - }; const Cache = jest.genMockFn().mockImplementation(function() { this._maps = Object.create(null); @@ -109,7 +103,6 @@ describe('DependencyGraph', function() { defaults = { assetExts: ['png', 'jpg'], cache: new Cache(), - fileWatcher, forceNodeFilesystemAPI: true, providesModuleNodeModules: [ 'haste-fbjs', @@ -4825,7 +4818,6 @@ describe('DependencyGraph', function() { }); describe('file watch updating', function() { - var triggerFileChange; var mockStat = { isDirectory: () => false, }; @@ -4836,21 +4828,6 @@ describe('DependencyGraph', function() { beforeEach(function() { process.platform = 'linux'; DependencyGraph = require('../index'); - - var callbacks = []; - triggerFileChange = (...args) => - callbacks.map(callback => callback(...args)); - - defaults.fileWatcher = { - on: function(eventType, callback) { - if (eventType !== 'all') { - throw new Error('Can only handle "all" event in watcher.'); - } - callbacks.push(callback); - return this; - }, - isWatchman: () => Promise.resolve(false), - }; }); afterEach(function() { @@ -4891,7 +4868,7 @@ describe('DependencyGraph', function() { return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { filesystem.root['index.js'] = filesystem.root['index.js'].replace('require("foo")', ''); - triggerFileChange('change', 'index.js', root, mockStat); + dgraph.processFileChange('change', root + '/index.js', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) .toEqual([ @@ -4954,7 +4931,7 @@ describe('DependencyGraph', function() { return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { filesystem.root['index.js'] = filesystem.root['index.js'].replace('require("foo")', ''); - triggerFileChange('change', 'index.js', root, mockStat); + dgraph.processFileChange('change', root + '/index.js', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) .toEqual([ @@ -5016,7 +4993,7 @@ describe('DependencyGraph', function() { }); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { delete filesystem.root.foo; - triggerFileChange('delete', 'foo.js', root); + dgraph.processFileChange('delete', root + '/foo.js'); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) .toEqual([ @@ -5083,10 +5060,10 @@ describe('DependencyGraph', function() { ' */', 'require("foo")', ].join('\n'); - triggerFileChange('add', 'bar.js', root, mockStat); + dgraph.processFileChange('add', root + '/bar.js', mockStat); filesystem.root.aPackage['main.js'] = 'require("bar")'; - triggerFileChange('change', 'aPackage/main.js', root, mockStat); + dgraph.processFileChange('change', root + '/aPackage/main.js', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) @@ -5175,7 +5152,7 @@ describe('DependencyGraph', function() { ]); filesystem.root['foo.png'] = ''; - triggerFileChange('add', 'foo.png', root, mockStat); + dgraph.processFileChange('add', root + '/foo.png', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps2) { expect(deps2) @@ -5245,7 +5222,7 @@ describe('DependencyGraph', function() { ]); filesystem.root['foo.png'] = ''; - triggerFileChange('add', 'foo.png', root, mockStat); + dgraph.processFileChange('add', root + '/foo.png', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps2) { expect(deps2) @@ -5277,171 +5254,6 @@ describe('DependencyGraph', function() { }); }); - it('runs changes through ignore filter', function() { - var root = '/root'; - var filesystem = setMockFileSystem({ - 'root': { - 'index.js': [ - '/**', - ' * @providesModule index', - ' */', - 'require("aPackage")', - 'require("foo")', - ].join('\n'), - 'foo.js': [ - '/**', - ' * @providesModule foo', - ' */', - 'require("aPackage")', - ].join('\n'), - 'aPackage': { - 'package.json': JSON.stringify({ - name: 'aPackage', - main: 'main.js', - }), - 'main.js': 'main', - }, - }, - }); - - var dgraph = new DependencyGraph({ - ...defaults, - roots: [root], - ignoreFilePath: function(filePath) { - if (filePath === '/root/bar.js') { - return true; - } - return false; - }, - }); - return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { - filesystem.root['bar.js'] = [ - '/**', - ' * @providesModule bar', - ' */', - 'require("foo")', - ].join('\n'); - triggerFileChange('add', 'bar.js', root, mockStat); - - filesystem.root.aPackage['main.js'] = 'require("bar")'; - triggerFileChange('change', 'aPackage/main.js', root, mockStat); - - return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { - expect(deps) - .toEqual([ - { - id: 'index', - path: '/root/index.js', - dependencies: ['aPackage', 'foo'], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - { - id: 'aPackage/main.js', - path: '/root/aPackage/main.js', - dependencies: ['bar'], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - { - id: 'foo', - path: '/root/foo.js', - dependencies: ['aPackage'], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - ]); - }); - }); - }); - - it('should ignore directory updates', function() { - var root = '/root'; - setMockFileSystem({ - 'root': { - 'index.js': [ - '/**', - ' * @providesModule index', - ' */', - 'require("aPackage")', - 'require("foo")', - ].join('\n'), - 'foo.js': [ - '/**', - ' * @providesModule foo', - ' */', - 'require("aPackage")', - ].join('\n'), - 'aPackage': { - 'package.json': JSON.stringify({ - name: 'aPackage', - main: 'main.js', - }), - 'main.js': 'main', - }, - }, - }); - var dgraph = new DependencyGraph({ - ...defaults, - roots: [root], - }); - return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { - triggerFileChange('change', 'aPackage', '/root', { - isDirectory: () => true, - }); - return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { - expect(deps) - .toEqual([ - { - id: 'index', - path: '/root/index.js', - dependencies: ['aPackage', 'foo'], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - { - id: 'aPackage/main.js', - path: '/root/aPackage/main.js', - dependencies: [], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - { - id: 'foo', - path: '/root/foo.js', - dependencies: ['aPackage'], - isAsset: false, - isAsset_DEPRECATED: false, - isJSON: false, - isPolyfill: false, - resolution: undefined, - resolveDependency: undefined, - }, - ]); - }); - }); - }); - it('changes to browser field', function() { var root = '/root'; var filesystem = setMockFileSystem({ @@ -5473,7 +5285,7 @@ describe('DependencyGraph', function() { main: 'main.js', browser: 'browser.js', }); - triggerFileChange('change', 'package.json', '/root/aPackage', mockStat); + dgraph.processFileChange('change', root + '/aPackage/package.json', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) @@ -5535,7 +5347,7 @@ describe('DependencyGraph', function() { name: 'bPackage', main: 'main.js', }); - triggerFileChange('change', 'package.json', '/root/aPackage', mockStat); + dgraph.processFileChange('change', root + '/aPackage/package.json', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { expect(deps) @@ -5630,7 +5442,7 @@ describe('DependencyGraph', function() { ]); filesystem.root.node_modules.foo['main.js'] = 'lol'; - triggerFileChange('change', 'main.js', '/root/node_modules/foo', mockStat); + dgraph.processFileChange('change', root + '/node_modules/foo/main.js', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps2) { expect(deps2) @@ -5695,7 +5507,7 @@ describe('DependencyGraph', function() { main: 'main.js', browser: 'browser.js', }); - triggerFileChange('change', 'package.json', '/root/node_modules/foo', mockStat); + dgraph.processFileChange('change', root + '/node_modules/foo/package.json', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps2) { expect(deps2) @@ -5752,7 +5564,7 @@ describe('DependencyGraph', function() { }); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function() { - triggerFileChange('add', 'index.js', root, mockStat); + dgraph.processFileChange('add', root + '/index.js', mockStat); return getOrderedDependenciesAsJSON(dgraph, '/root/index.js'); }); }); diff --git a/packager/react-packager/src/node-haste/__tests__/Module-test.js b/packager/react-packager/src/node-haste/__tests__/Module-test.js index f7aad9a09a1ef5..53afd8a56bb5e5 100644 --- a/packager/react-packager/src/node-haste/__tests__/Module-test.js +++ b/packager/react-packager/src/node-haste/__tests__/Module-test.js @@ -47,10 +47,6 @@ function mockIndexFile(indexJs) { } describe('Module', () => { - const fileWatcher = { - on: () => this, - isWatchman: () => Promise.resolve(false), - }; const fileName = '/root/index.js'; let cache, fastfs; @@ -85,7 +81,6 @@ describe('Module', () => { new Fastfs( 'test', ['/root'], - fileWatcher, ['/root/index.js', '/root/package.json'], {ignore: []}, ); @@ -119,22 +114,22 @@ describe('Module', () => { mockIndexFile(source); }); - pit('extracts the module name from the header', () => + it('extracts the module name from the header', () => module.getName().then(name => expect(name).toEqual(moduleId)) ); - pit('identifies the module as haste module', () => + it('identifies the module as haste module', () => module.isHaste().then(isHaste => expect(isHaste).toBe(true)) ); - pit('does not transform the file in order to access the name', () => { + it('does not transform the file in order to access the name', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).getName() .then(() => expect(transformCode).not.toBeCalled()); }); - pit('does not transform the file in order to access the haste status', () => { + it('does not transform the file in order to access the haste status', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).isHaste() @@ -147,22 +142,22 @@ describe('Module', () => { mockIndexFile(source.replace(/@providesModule/, '@provides')); }); - pit('extracts the module name from the header if it has a @provides annotation', () => + it('extracts the module name from the header if it has a @provides annotation', () => module.getName().then(name => expect(name).toEqual(moduleId)) ); - pit('identifies the module as haste module', () => + it('identifies the module as haste module', () => module.isHaste().then(isHaste => expect(isHaste).toBe(true)) ); - pit('does not transform the file in order to access the name', () => { + it('does not transform the file in order to access the name', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).getName() .then(() => expect(transformCode).not.toBeCalled()); }); - pit('does not transform the file in order to access the haste status', () => { + it('does not transform the file in order to access the haste status', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).isHaste() @@ -175,22 +170,22 @@ describe('Module', () => { mockIndexFile('arbitrary(code);'); }); - pit('uses the file name as module name', () => + it('uses the file name as module name', () => module.getName().then(name => expect(name).toEqual(fileName)) ); - pit('does not identify the module as haste module', () => + it('does not identify the module as haste module', () => module.isHaste().then(isHaste => expect(isHaste).toBe(false)) ); - pit('does not transform the file in order to access the name', () => { + it('does not transform the file in order to access the name', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).getName() .then(() => expect(transformCode).not.toBeCalled()); }); - pit('does not transform the file in order to access the haste status', () => { + it('does not transform the file in order to access the haste status', () => { const transformCode = jest.genMockFn().mockReturnValue(Promise.resolve()); return createModule({transformCode}).isHaste() @@ -205,12 +200,12 @@ describe('Module', () => { mockIndexFile(fileContents); }); - pit('exposes file contents as `code` property on the data exposed by `read()`', () => + it('exposes file contents as `code` property on the data exposed by `read()`', () => createModule().read().then(({code}) => expect(code).toBe(fileContents)) ); - pit('exposes file contents via the `getCode()` method', () => + it('exposes file contents via the `getCode()` method', () => createModule().getCode().then(code => expect(code).toBe(fileContents)) ); @@ -241,7 +236,7 @@ describe('Module', () => { mockIndexFile(fileContents); }); - pit('passes the module and file contents to the transform function when reading', () => { + it('passes the module and file contents to the transform function when reading', () => { const module = createModule({transformCode}); return module.read() .then(() => { @@ -249,7 +244,7 @@ describe('Module', () => { }); }); - pit('passes any additional options to the transform function when reading', () => { + it('passes any additional options to the transform function when reading', () => { const module = createModule({transformCode}); const transformOptions = {arbitrary: Object()}; return module.read(transformOptions) @@ -258,7 +253,7 @@ describe('Module', () => { ); }); - pit('passes the module and file contents to the transform if the file is annotated with @extern', () => { + it('passes module and file contents if the file is annotated with @extern', () => { const module = createModule({transformCode}); const customFileContents = ` /** @@ -271,7 +266,7 @@ describe('Module', () => { }); }); - pit('passes the module and file contents to the transform for JSON files', () => { + it('passes the module and file contents to the transform for JSON files', () => { mockPackageFile(); const module = createJSONModule({transformCode}); return module.read().then(() => { @@ -279,7 +274,7 @@ describe('Module', () => { }); }); - pit('does not extend the passed options object if the file is annotated with @extern', () => { + it('does not extend the passed options object if the file is annotated with @extern', () => { const module = createModule({transformCode}); const customFileContents = ` /** @@ -295,7 +290,7 @@ describe('Module', () => { }); }); - pit('does not extend the passed options object for JSON files', () => { + it('does not extend the passed options object for JSON files', () => { mockPackageFile(); const module = createJSONModule({transformCode}); const options = {arbitrary: 'foo'}; @@ -306,7 +301,7 @@ describe('Module', () => { }); }); - pit('uses dependencies that `transformCode` resolves to, instead of extracting them', () => { + it('uses dependencies that `transformCode` resolves to, instead of extracting them', () => { const mockedDependencies = ['foo', 'bar']; transformResult = { code: exampleCode, @@ -319,7 +314,7 @@ describe('Module', () => { }); }); - pit('forwards all additional properties of the result provided by `transformCode`', () => { + it('forwards all additional properties of the result provided by `transformCode`', () => { transformResult = { code: exampleCode, arbitrary: 'arbitrary', @@ -334,7 +329,7 @@ describe('Module', () => { }); }); - pit('does not store anything but dependencies if the `cacheTransformResults` option is disabled', () => { + it('only stores dependencies if `cacheTransformResults` option is disabled', () => { transformResult = { code: exampleCode, arbitrary: 'arbitrary', @@ -354,7 +349,7 @@ describe('Module', () => { }); }); - pit('stores all things if options is undefined', () => { + it('stores all things if options is undefined', () => { transformResult = { code: exampleCode, arbitrary: 'arbitrary', @@ -370,7 +365,7 @@ describe('Module', () => { }); }); - pit('exposes the transformed code rather than the raw file contents', () => { + it('exposes the transformed code rather than the raw file contents', () => { transformResult = {code: exampleCode}; const module = createModule({transformCode}); return Promise.all([module.read(), module.getCode()]) @@ -380,13 +375,13 @@ describe('Module', () => { }); }); - pit('exposes the raw file contents as `source` property', () => { + it('exposes the raw file contents as `source` property', () => { const module = createModule({transformCode}); return module.read() .then(data => expect(data.source).toBe(fileContents)); }); - pit('exposes a source map returned by the transform', () => { + it('exposes a source map returned by the transform', () => { const map = {version: 3}; transformResult = {map, code: exampleCode}; const module = createModule({transformCode}); @@ -397,7 +392,7 @@ describe('Module', () => { }); }); - pit('caches the transform result for the same transform options', () => { + it('caches the transform result for the same transform options', () => { let module = createModule({transformCode}); return module.read() .then(() => { @@ -412,7 +407,7 @@ describe('Module', () => { }); }); - pit('triggers a new transform for different transform options', () => { + it('triggers a new transform for different transform options', () => { const module = createModule({transformCode}); return module.read({foo: 1}) .then(() => { @@ -424,7 +419,7 @@ describe('Module', () => { }); }); - pit('triggers a new transform for different source code', () => { + it('triggers a new transform for different source code', () => { let module = createModule({transformCode}); return module.read() .then(() => { @@ -440,7 +435,7 @@ describe('Module', () => { }); }); - pit('triggers a new transform for different transform cache key', () => { + it('triggers a new transform for different transform cache key', () => { let module = createModule({transformCode}); return module.read() .then(() => { diff --git a/packager/react-packager/src/node-haste/fastfs.js b/packager/react-packager/src/node-haste/fastfs.js index e8e8220b5c277c..58f381e2ade29e 100644 --- a/packager/react-packager/src/node-haste/fastfs.js +++ b/packager/react-packager/src/node-haste/fastfs.js @@ -18,10 +18,6 @@ const {EventEmitter} = require('events'); const NOT_FOUND_IN_ROOTS = 'NotFoundInRootsError'; -interface FileWatcher { - on(event: 'all', handler: (type: string, filePath: string, rootPath: string, fstat: fs.Stats) => void): void, -} - const { createActionStartEntry, createActionEndEntry, @@ -32,7 +28,6 @@ const { class Fastfs extends EventEmitter { _name: string; - _fileWatcher: FileWatcher; _ignore: (filePath: string) => boolean; _roots: Array; _fastPaths: {[filePath: string]: File}; @@ -40,7 +35,6 @@ class Fastfs extends EventEmitter { constructor( name: string, roots: Array, - fileWatcher: FileWatcher, files: Array, {ignore}: { ignore: (filePath: string) => boolean, @@ -48,7 +42,6 @@ class Fastfs extends EventEmitter { ) { super(); this._name = name; - this._fileWatcher = fileWatcher; this._ignore = ignore; this._roots = roots.map(root => { // If the path ends in a separator ("/"), remove it to make string @@ -82,10 +75,6 @@ class Fastfs extends EventEmitter { }); print(log(createActionEndEntry(buildingInMemoryFSLogEntry))); - - if (this._fileWatcher) { - this._fileWatcher.on('all', this._processFileChange.bind(this)); - } } stat(filePath: string) { @@ -205,33 +194,23 @@ class Fastfs extends EventEmitter { return this._fastPaths[filePath]; } - _processFileChange(type, filePath, rootPath, fstat) { - const absPath = path.join(rootPath, filePath); - if (this._ignore(absPath) || (fstat && fstat.isDirectory())) { - return; - } - - // Make sure this event belongs to one of our roots. - const root = this._getRoot(absPath); - if (!root) { - return; - } - + processFileChange(type: string, filePath: string) { if (type === 'delete' || type === 'change') { - const file = this._getFile(absPath); + const file = this._getFile(filePath); if (file) { file.remove(); } } - delete this._fastPaths[path.resolve(absPath)]; + delete this._fastPaths[path.resolve(filePath)]; if (type !== 'delete') { - const file = new File(absPath, false); - root.addChild(file, this._fastPaths); + const file = new File(filePath, false); + const root = this._getRoot(filePath); + if (root) { + root.addChild(file, this._fastPaths); + } } - - this.emit('change', type, filePath, rootPath, fstat); } } diff --git a/packager/react-packager/src/node-haste/index.js b/packager/react-packager/src/node-haste/index.js index 9946e90aae3975..1397723bdcff44 100644 --- a/packager/react-packager/src/node-haste/index.js +++ b/packager/react-packager/src/node-haste/index.js @@ -15,7 +15,6 @@ const Cache = require('./Cache'); const DependencyGraphHelpers = require('./DependencyGraph/DependencyGraphHelpers'); const DeprecatedAssetMap = require('./DependencyGraph/DeprecatedAssetMap'); const Fastfs = require('./fastfs'); -const FileWatcher = require('./FileWatcher'); const HasteMap = require('./DependencyGraph/HasteMap'); const JestHasteMap = require('jest-haste-map'); const Module = require('./Module'); @@ -47,12 +46,16 @@ const { print, } = require('../Logger'); +const escapePath = (p: string) => { + return (path.sep === '\\') ? p.replace(/(\/|\\(?!\.))/g, '\\\\') : p; +}; + class DependencyGraph { _opts: { roots: Array, ignoreFilePath: (filePath: string) => boolean, - fileWatcher: FileWatcher, + watch: boolean, forceNodeFilesystemAPI: boolean, assetRoots_DEPRECATED: Array, assetExts: Array, @@ -71,21 +74,23 @@ class DependencyGraph { maxWorkers: number, resetCache: boolean, }; - _cache: Cache; _assetDependencies: mixed; - _helpers: DependencyGraphHelpers; + _assetPattern: RegExp; + _cache: Cache; + _deprecatedAssetMap: DeprecatedAssetMap; _fastfs: Fastfs; - _moduleCache: ModuleCache; + _haste: JestHasteMap; _hasteMap: HasteMap; - _deprecatedAssetMap: DeprecatedAssetMap; _hasteMapError: ?Error; + _helpers: DependencyGraphHelpers; + _moduleCache: ModuleCache; - _loading: ?Promise; + _loading: Promise; constructor({ roots, ignoreFilePath, - fileWatcher, + watch, forceNodeFilesystemAPI, assetRoots_DEPRECATED, assetExts, @@ -109,7 +114,7 @@ class DependencyGraph { }: { roots: Array, ignoreFilePath: (filePath: string) => boolean, - fileWatcher: FileWatcher, + watch: boolean, forceNodeFilesystemAPI?: boolean, assetRoots_DEPRECATED: Array, assetExts: Array, @@ -133,7 +138,7 @@ class DependencyGraph { this._opts = { roots, ignoreFilePath: ignoreFilePath || (() => {}), - fileWatcher, + watch: !!watch, forceNodeFilesystemAPI: !!forceNodeFilesystemAPI, assetRoots_DEPRECATED: assetRoots_DEPRECATED || [], assetExts: assetExts || [], @@ -155,6 +160,9 @@ class DependencyGraph { maxWorkers, resetCache, }; + this._assetPattern = + new RegExp('^' + this._opts.assetRoots_DEPRECATED.map(escapePath).join('|')); + this._cache = cache; this._assetDependencies = assetDependencies; this._helpers = new DependencyGraphHelpers(this._opts); @@ -167,7 +175,7 @@ class DependencyGraph { } const mw = this._opts.maxWorkers; - const haste = new JestHasteMap({ + this._haste = new JestHasteMap({ extensions: this._opts.extensions.concat(this._opts.assetExts), forceNodeFilesystemAPI: this._opts.forceNodeFilesystemAPI, ignorePattern: {test: this._opts.ignoreFilePath}, @@ -180,9 +188,10 @@ class DependencyGraph { retainAllFiles: true, roots: this._opts.roots.concat(this._opts.assetRoots_DEPRECATED), useWatchman: this._opts.useWatchman, + watch: this._opts.watch, }); - this._loading = haste.build().then(hasteMap => { + this._loading = this._haste.build().then(hasteMap => { const initializingPackagerLogEntry = print(log(createActionStartEntry('Initializing Packager'))); @@ -191,15 +200,12 @@ class DependencyGraph { this._fastfs = new Fastfs( 'JavaScript', this._opts.roots, - this._opts.fileWatcher, hasteFSFiles, { ignore: this._opts.ignoreFilePath, } ); - this._fastfs.on('change', this._processFileChange.bind(this)); - this._moduleCache = new ModuleCache({ fastfs: this._fastfs, cache: this._cache, @@ -219,14 +225,7 @@ class DependencyGraph { platforms: this._opts.platforms, }); - const escapePath = (p: string) => { - return (path.sep === '\\') ? p.replace(/(\/|\\(?!\.))/g, '\\\\') : p; - }; - - const assetPattern = - new RegExp('^' + this._opts.assetRoots_DEPRECATED.map(escapePath).join('|')); - - const assetFiles = hasteMap.hasteFS.matchFiles(assetPattern); + const assetFiles = hasteMap.hasteFS.matchFiles(this._assetPattern); this._deprecatedAssetMap = new DeprecatedAssetMap({ helpers: this._helpers, @@ -235,11 +234,11 @@ class DependencyGraph { files: assetFiles, }); - this._fastfs.on('change', (type, filePath, root, fstat) => { - if (assetPattern.test(path.join(root, filePath))) { - this._deprecatedAssetMap.processFileChange(type, filePath, root, fstat); - } - }); + this._haste.on('change', ({eventsQueue}) => + eventsQueue.forEach(({type, filePath, stat}) => + this.processFileChange(type, filePath, stat) + ) + ); const buildingHasteMapLogEntry = print(log(createActionStartEntry('Building Haste Map'))); @@ -279,6 +278,10 @@ class DependencyGraph { return this._fastfs; } + getWatcher() { + return this._haste; + } + /** * Returns the module object for the given path. */ @@ -365,21 +368,16 @@ class DependencyGraph { ); } - _processFileChange(type, filePath, root, fstat) { - const absPath = path.join(root, filePath); - if (fstat && fstat.isDirectory() || - this._opts.ignoreFilePath(absPath) || - this._helpers.isNodeModulesDir(absPath)) { - return; + processFileChange(type: string, filePath: string, stat: Object) { + this._fastfs.processFileChange(type, filePath, stat); + this._moduleCache.processFileChange(type, filePath, stat); + if (this._assetPattern.test(filePath)) { + this._deprecatedAssetMap.processFileChange(type, filePath, stat); } - // Ok, this is some tricky promise code. Our requirements are: - // * we need to report back failures - // * failures shouldn't block recovery - // * Errors can leave `hasteMap` in an incorrect state, and we need to rebuild - // After we process a file change we record any errors which will also be - // reported via the next request. On the next file change, we'll see that - // we are in an error state and we should decide to do a full rebuild. + // This code reports failures but doesn't block recovery in the dev server + // mode. When the hasteMap is left in an incorrect state, we'll rebuild when + // the next file changes. const resolve = () => { if (this._hasteMapError) { console.warn( @@ -391,13 +389,14 @@ class DependencyGraph { // Rebuild the entire map if last change resulted in an error. this._loading = this._hasteMap.build(); } else { - this._loading = this._hasteMap.processFileChange(type, absPath); - this._loading.catch((e) => {this._hasteMapError = e;}); + this._loading = this._hasteMap.processFileChange(type, filePath); + this._loading.catch(error => { + this._hasteMapError = error; + }); } return this._loading; }; - /* $FlowFixMe: there is a risk this happen before we assign that - * variable in the load() function. */ + this._loading = this._loading.then(resolve, resolve); } @@ -411,7 +410,6 @@ class DependencyGraph { static Cache; static Fastfs; - static FileWatcher; static Module; static Polyfill; static getAssetDataFromName; @@ -424,7 +422,6 @@ class DependencyGraph { Object.assign(DependencyGraph, { Cache, Fastfs, - FileWatcher, Module, Polyfill, getAssetDataFromName,