[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:
parent
9eb5151bd2
commit
3388d8fe79
|
@ -118,14 +118,14 @@ class Bundler {
|
|||
return this._cache.end();
|
||||
}
|
||||
|
||||
bundle(main, runModule, sourceMapUrl, isDev) {
|
||||
bundle(main, runModule, sourceMapUrl, isDev, platform) {
|
||||
const bundle = new Bundle(sourceMapUrl);
|
||||
|
||||
const transformModule = this._transformModule.bind(this, bundle);
|
||||
const findEventId = Activity.startEvent('find dependencies');
|
||||
let transformEventId;
|
||||
|
||||
return this.getDependencies(main, isDev).then((result) => {
|
||||
return this.getDependencies(main, isDev, platform).then((result) => {
|
||||
Activity.endEvent(findEventId);
|
||||
transformEventId = Activity.startEvent('transform');
|
||||
|
||||
|
@ -149,8 +149,8 @@ class Bundler {
|
|||
this._transformer.invalidateFile(filePath);
|
||||
}
|
||||
|
||||
getDependencies(main, isDev) {
|
||||
return this._resolver.getDependencies(main, { dev: isDev });
|
||||
getDependencies(main, isDev, platform) {
|
||||
return this._resolver.getDependencies(main, { dev: isDev, platform });
|
||||
}
|
||||
|
||||
_transformModule(bundle, module) {
|
||||
|
|
|
@ -69,7 +69,7 @@ class DependencyGraph {
|
|||
constructor(options) {
|
||||
this._opts = validateOpts(options);
|
||||
this._hasteMap = Object.create(null);
|
||||
this._immediateResolutionCache = Object.create(null);
|
||||
this._resetResolutionCache();
|
||||
this._cache = this._opts.cache;
|
||||
this.load();
|
||||
}
|
||||
|
@ -109,6 +109,20 @@ class DependencyGraph {
|
|||
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) {
|
||||
const resHash = resolutionHash(fromModule.path, toModuleName);
|
||||
|
||||
|
@ -250,12 +264,15 @@ class DependencyGraph {
|
|||
);
|
||||
}
|
||||
|
||||
// `platformExt` could be set in the `setup` method.
|
||||
if (!this._platformExt) {
|
||||
const platformExt = getPlatformExt(entryPath);
|
||||
if (platformExt && this._opts.platforms.indexOf(platformExt) > -1) {
|
||||
this._platformExt = platformExt;
|
||||
} else {
|
||||
this._platformExt = null;
|
||||
}
|
||||
}
|
||||
|
||||
return this._moduleCache.getModule(absolutePath);
|
||||
}
|
||||
|
@ -563,7 +580,7 @@ class DependencyGraph {
|
|||
_processFileChange(type, filePath, root, fstat) {
|
||||
// It's really hard to invalidate the right module resolution cache
|
||||
// so we just blow it up with every file change.
|
||||
this._immediateResolutionCache = Object.create(null);
|
||||
this._resetResolutionCache();
|
||||
|
||||
const absPath = path.join(root, filePath);
|
||||
if ((fstat && fstat.isDirectory()) ||
|
||||
|
@ -599,6 +616,10 @@ class DependencyGraph {
|
|||
});
|
||||
}
|
||||
}
|
||||
|
||||
_resetResolutionCache() {
|
||||
this._immediateResolutionCache = Object.create(null);
|
||||
}
|
||||
}
|
||||
|
||||
function assetName(file, ext) {
|
||||
|
|
|
@ -77,6 +77,10 @@ var getDependenciesValidateOpts = declareOpts({
|
|||
type: 'boolean',
|
||||
default: true,
|
||||
},
|
||||
platform: {
|
||||
type: 'string',
|
||||
required: false,
|
||||
},
|
||||
});
|
||||
|
||||
HasteDependencyResolver.prototype.getDependencies = function(main, options) {
|
||||
|
@ -85,13 +89,12 @@ HasteDependencyResolver.prototype.getDependencies = function(main, options) {
|
|||
var depGraph = this._depGraph;
|
||||
var self = this;
|
||||
|
||||
return depGraph
|
||||
.load()
|
||||
.then(() => Promise.all([
|
||||
depGraph.setup({ platform: opts.platform });
|
||||
|
||||
return Promise.all([
|
||||
depGraph.getOrderedDependencies(main),
|
||||
depGraph.getAsyncDependencies(main),
|
||||
]))
|
||||
.then(
|
||||
]).then(
|
||||
([dependencies, asyncDependencies]) => dependencies[0].getName().then(
|
||||
mainModuleId => {
|
||||
self._prependPolyfillDependencies(
|
||||
|
|
|
@ -110,7 +110,24 @@ describe('processRequest', () => {
|
|||
'index.ios.js',
|
||||
true,
|
||||
'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',
|
||||
true,
|
||||
undefined,
|
||||
true
|
||||
true,
|
||||
undefined
|
||||
);
|
||||
});
|
||||
});
|
||||
|
@ -253,7 +271,8 @@ describe('processRequest', () => {
|
|||
'path/to/foo.js',
|
||||
false,
|
||||
'/path/to/foo.map',
|
||||
false
|
||||
false,
|
||||
undefined
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
@ -88,6 +88,10 @@ const bundleOpts = declareOpts({
|
|||
type: 'boolean',
|
||||
default: false,
|
||||
},
|
||||
platform: {
|
||||
type: 'string',
|
||||
required: false,
|
||||
}
|
||||
});
|
||||
|
||||
class Server {
|
||||
|
@ -152,12 +156,12 @@ class Server {
|
|||
|
||||
buildBundle(options) {
|
||||
const opts = bundleOpts(options);
|
||||
|
||||
return this._bundler.bundle(
|
||||
opts.entryFile,
|
||||
opts.runModule,
|
||||
opts.sourceMapUrl,
|
||||
opts.dev
|
||||
opts.dev,
|
||||
opts.platform
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -425,6 +429,7 @@ class Server {
|
|||
'inlineSourceMap',
|
||||
false
|
||||
),
|
||||
platform: urlObj.query.platform,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue