[reat-packager] Switch platform resolution based on query param

Summary:
Currently the platform selection is controlled by the blacklist. However, since we want to use the same server instance for cross-platform development, we need this to be controlled per request.

One outstanding issue, is that the DependencyGraph class wasn't designed that way and it doesn't have a per-request state. This means that with the current design race conditions is possible. If we got a request for a different platfrom while processing the previous request, we may change the outcome of the previous request.

To fix this a larger refactor is needed. I'll follow up a diff to do that.

Finally, so I don't break the universe like last time, I'll leave it up to the RN guys to update the call sites.
This commit is contained in:
Amjad Masad 2015-08-15 12:29:35 -07:00
parent cc01b6daf4
commit 3555afb66d
5 changed files with 85 additions and 37 deletions

View File

@ -118,14 +118,14 @@ class Bundler {
return this._cache.end(); return this._cache.end();
} }
bundle(main, runModule, sourceMapUrl, isDev) { bundle(main, runModule, sourceMapUrl, isDev, platform) {
const bundle = new Bundle(sourceMapUrl); const bundle = new Bundle(sourceMapUrl);
const transformModule = this._transformModule.bind(this, bundle); const transformModule = this._transformModule.bind(this, bundle);
const findEventId = Activity.startEvent('find dependencies'); const findEventId = Activity.startEvent('find dependencies');
let transformEventId; let transformEventId;
return this.getDependencies(main, isDev).then((result) => { return this.getDependencies(main, isDev, platform).then((result) => {
Activity.endEvent(findEventId); Activity.endEvent(findEventId);
transformEventId = Activity.startEvent('transform'); transformEventId = Activity.startEvent('transform');
@ -149,8 +149,8 @@ class Bundler {
this._transformer.invalidateFile(filePath); this._transformer.invalidateFile(filePath);
} }
getDependencies(main, isDev) { getDependencies(main, isDev, platform) {
return this._resolver.getDependencies(main, { dev: isDev }); return this._resolver.getDependencies(main, { dev: isDev, platform });
} }
_transformModule(bundle, module) { _transformModule(bundle, module) {

View File

@ -69,7 +69,7 @@ class DependencyGraph {
constructor(options) { constructor(options) {
this._opts = validateOpts(options); this._opts = validateOpts(options);
this._hasteMap = Object.create(null); this._hasteMap = Object.create(null);
this._immediateResolutionCache = Object.create(null); this._resetResolutionCache();
this._cache = this._opts.cache; this._cache = this._opts.cache;
this.load(); this.load();
} }
@ -109,6 +109,20 @@ class DependencyGraph {
return this._loading; return this._loading;
} }
setup({ platform }) {
if (platform && this._opts.platforms.indexOf(platform) === -1) {
throw new Error('Unrecognized platform: ' + platform);
}
// TODO(amasad): This is a potential race condition. Mutliple requests could
// interfere with each other. This needs a refactor to fix -- which will
// follow this diff.
if (this._platformExt !== platform) {
this._resetResolutionCache();
}
this._platformExt = platform;
}
resolveDependency(fromModule, toModuleName) { resolveDependency(fromModule, toModuleName) {
const resHash = resolutionHash(fromModule.path, toModuleName); const resHash = resolutionHash(fromModule.path, toModuleName);
@ -250,11 +264,14 @@ class DependencyGraph {
); );
} }
const platformExt = getPlatformExt(entryPath); // `platformExt` could be set in the `setup` method.
if (platformExt && this._opts.platforms.indexOf(platformExt) > -1) { if (!this._platformExt) {
this._platformExt = platformExt; const platformExt = getPlatformExt(entryPath);
} else { if (platformExt && this._opts.platforms.indexOf(platformExt) > -1) {
this._platformExt = null; this._platformExt = platformExt;
} else {
this._platformExt = null;
}
} }
return this._moduleCache.getModule(absolutePath); return this._moduleCache.getModule(absolutePath);
@ -563,7 +580,7 @@ class DependencyGraph {
_processFileChange(type, filePath, root, fstat) { _processFileChange(type, filePath, root, fstat) {
// It's really hard to invalidate the right module resolution cache // It's really hard to invalidate the right module resolution cache
// so we just blow it up with every file change. // so we just blow it up with every file change.
this._immediateResolutionCache = Object.create(null); this._resetResolutionCache();
const absPath = path.join(root, filePath); const absPath = path.join(root, filePath);
if ((fstat && fstat.isDirectory()) || if ((fstat && fstat.isDirectory()) ||
@ -599,6 +616,10 @@ class DependencyGraph {
}); });
} }
} }
_resetResolutionCache() {
this._immediateResolutionCache = Object.create(null);
}
} }
function assetName(file, ext) { function assetName(file, ext) {

View File

@ -77,6 +77,10 @@ var getDependenciesValidateOpts = declareOpts({
type: 'boolean', type: 'boolean',
default: true, default: true,
}, },
platform: {
type: 'string',
required: false,
},
}); });
HasteDependencyResolver.prototype.getDependencies = function(main, options) { HasteDependencyResolver.prototype.getDependencies = function(main, options) {
@ -85,28 +89,27 @@ HasteDependencyResolver.prototype.getDependencies = function(main, options) {
var depGraph = this._depGraph; var depGraph = this._depGraph;
var self = this; var self = this;
return depGraph depGraph.setup({ platform: opts.platform });
.load()
.then(() => Promise.all([
depGraph.getOrderedDependencies(main),
depGraph.getAsyncDependencies(main),
]))
.then(
([dependencies, asyncDependencies]) => dependencies[0].getName().then(
mainModuleId => {
self._prependPolyfillDependencies(
dependencies,
opts.dev,
);
return { return Promise.all([
mainModuleId, depGraph.getOrderedDependencies(main),
dependencies, depGraph.getAsyncDependencies(main),
asyncDependencies, ]).then(
}; ([dependencies, asyncDependencies]) => dependencies[0].getName().then(
} mainModuleId => {
) self._prependPolyfillDependencies(
); dependencies,
opts.dev,
);
return {
mainModuleId,
dependencies,
asyncDependencies,
};
}
)
);
}; };
HasteDependencyResolver.prototype._prependPolyfillDependencies = function( HasteDependencyResolver.prototype._prependPolyfillDependencies = function(

View File

@ -110,7 +110,24 @@ describe('processRequest', () => {
'index.ios.js', 'index.ios.js',
true, true,
'index.ios.includeRequire.map', 'index.ios.includeRequire.map',
true true,
undefined
);
});
});
pit('passes in the platform param', function() {
return makeRequest(
requestHandler,
'index.bundle?platform=ios'
).then(function(response) {
expect(response).toEqual('this is the source');
expect(Bundler.prototype.bundle).toBeCalledWith(
'index.js',
true,
'index.map',
true,
'ios',
); );
}); });
}); });
@ -241,7 +258,8 @@ describe('processRequest', () => {
'foo file', 'foo file',
true, true,
undefined, undefined,
true true,
undefined
); );
}); });
}); });
@ -253,7 +271,8 @@ describe('processRequest', () => {
'path/to/foo.js', 'path/to/foo.js',
false, false,
'/path/to/foo.map', '/path/to/foo.map',
false false,
undefined
); );
}); });
}); });

View File

@ -88,6 +88,10 @@ const bundleOpts = declareOpts({
type: 'boolean', type: 'boolean',
default: false, default: false,
}, },
platform: {
type: 'string',
required: false,
}
}); });
class Server { class Server {
@ -152,12 +156,12 @@ class Server {
buildBundle(options) { buildBundle(options) {
const opts = bundleOpts(options); const opts = bundleOpts(options);
return this._bundler.bundle( return this._bundler.bundle(
opts.entryFile, opts.entryFile,
opts.runModule, opts.runModule,
opts.sourceMapUrl, opts.sourceMapUrl,
opts.dev opts.dev,
opts.platform
); );
} }
@ -425,6 +429,7 @@ class Server {
'inlineSourceMap', 'inlineSourceMap',
false false
), ),
platform: urlObj.query.platform,
}; };
} }