Skip to content

Commit

Permalink
[react-packager] Implement the browser field package.json spec
Browse files Browse the repository at this point in the history
Summary:
Apparently we promote using superagent but it doesn't actually work. See issues facebook#370 facebook#10 facebook#375
The main issue stopping it from working is the `browser` field in package.json which both browserify and webpack implments.
This implements most of the spec found here https://gist.github.com/defunctzombie/4339901
The only thing we're not implementing is the `module-name: false` part to ignore a module because it doesn't seem to be as used. If someone complains about it we'll implement it then.

Test Plan:
./runJestTests.sh
* npm install superagent in JS
* test using superagent
  • Loading branch information
Amjad Masad authored and oss sync committed Apr 12, 2015
1 parent d2750c1 commit dc4e8d1
Show file tree
Hide file tree
Showing 2 changed files with 333 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,296 @@ describe('DependencyGraph', function() {
]);
});
});

pit('should support simple browser field in packages', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: 'main.js',
browser: 'client.js',
}),
'main.js': 'some other code',
'client.js': 'some code',
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/client',
path: '/root/aPackage/client.js',
dependencies: []
},
]);
});
});

pit('should supportbrowser field in packages w/o .js ext', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: 'main.js',
browser: 'client',
}),
'main.js': 'some other code',
'client.js': 'some code',
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/client',
path: '/root/aPackage/client.js',
dependencies: []
},
]);
});
});

pit('should support mapping main in browser field json', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: './main.js',
browser: {
'./main.js': './client.js',
},
}),
'main.js': 'some other code',
'client.js': 'some code',
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/client',
path: '/root/aPackage/client.js',
dependencies: []
},
]);
});
});

pit('should work do correct browser mapping w/o js ext', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: './main.js',
browser: {
'./main': './client.js',
},
}),
'main.js': 'some other code',
'client.js': 'some code',
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/client',
path: '/root/aPackage/client.js',
dependencies: []
},
]);
});
});

pit('should support browser mapping of files', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
main: './main.js',
browser: {
'./main': './client.js',
'./node.js': './not-node.js',
'./not-browser': './browser.js',
'./dir/server.js': './dir/client',
},
}),
'main.js': 'some other code',
'client.js': 'require("./node")\nrequire("./dir/server.js")',
'not-node.js': 'require("./not-browser")',
'not-browser.js': 'require("./dir/server")',
'browser.js': 'some browser code',
'dir': {
'server.js': 'some node code',
'client.js': 'some browser code',
}
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/client',
path: '/root/aPackage/client.js',
dependencies: ['./node', './dir/server.js']
},
{ id: 'aPackage/not-node',
path: '/root/aPackage/not-node.js',
dependencies: ['./not-browser']
},
{ id: 'aPackage/browser',
path: '/root/aPackage/browser.js',
dependencies: []
},
{ id: 'aPackage/dir/client',
path: '/root/aPackage/dir/client.js',
dependencies: []
},
]);
});
});

pit('should support browser mapping for packages', function() {
var root = '/root';
fs.__setMockFilesystem({
'root': {
'index.js': [
'/**',
' * @providesModule index',
' */',
'require("aPackage")',
].join('\n'),
'aPackage': {
'package.json': JSON.stringify({
name: 'aPackage',
browser: {
'node-package': 'browser-package',
}
}),
'index.js': 'require("node-package")',
'node-package': {
'package.json': JSON.stringify({
'name': 'node-package',
}),
'index.js': 'some node code',
},
'browser-package': {
'package.json': JSON.stringify({
'name': 'browser-package',
}),
'index.js': 'some browser code',
},
}
}
});

var dgraph = new DependencyGraph({
roots: [root],
fileWatcher: fileWatcher
});
return dgraph.load().then(function() {
expect(dgraph.getOrderedDependencies('/root/index.js'))
.toEqual([
{ id: 'index', altId: '/root/index.js',
path: '/root/index.js',
dependencies: ['aPackage']
},
{ id: 'aPackage/index',
path: '/root/aPackage/index.js',
dependencies: ['node-package']
},
{ id: 'browser-package/index',
path: '/root/aPackage/browser-package/index.js',
dependencies: []
},
]);
});
});
});

describe('file watch updating', function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,22 @@ DependecyGraph.prototype.resolveDependency = function(
// Package relative modules starts with '.' or '..'.
if (depModuleId[0] !== '.') {

// 1. `depModuleId` is simply a top-level `providesModule`.
// 2. `depModuleId` is a package module but given the full path from the
// package, i.e. package_name/module_name
// Check if we need to map the dependency to something else via the
// `browser` field in package.json
var fromPackageJson = this._lookupPackage(fromModule.path);
if (fromPackageJson && fromPackageJson.browser &&
fromPackageJson.browser[depModuleId]) {
depModuleId = fromPackageJson.browser[depModuleId];
}

// `depModuleId` is simply a top-level `providesModule`.
// `depModuleId` is a package module but given the full path from the
// package, i.e. package_name/module_name
if (this._moduleById[sansExtJs(depModuleId)]) {
return this._moduleById[sansExtJs(depModuleId)];
}

// 3. `depModuleId` is a package and it's depending on the "main"
// resolution.
// `depModuleId` is a package and it's depending on the "main" resolution.
packageJson = this._packagesById[depModuleId];

// We are being forgiving here and raising an error because we could be
Expand All @@ -190,7 +197,25 @@ DependecyGraph.prototype.resolveDependency = function(
return null;
}

var main = packageJson.main || 'index';
var main;

// We prioritize the `browser` field if it's a module path.
if (typeof packageJson.browser === 'string') {
main = packageJson.browser;
} else {
main = packageJson.main || 'index';
}

// If there is a mapping for main in the `browser` field.
if (packageJson.browser && typeof packageJson.browser === 'object') {
var tmpMain = packageJson.browser[main] ||
packageJson.browser[withExtJs(main)] ||
packageJson.browser[sansExtJs(main)];
if (tmpMain) {
main = tmpMain;
}
}

modulePath = withExtJs(path.join(packageJson._root, main));
dep = this._graph[modulePath];

Expand All @@ -207,8 +232,7 @@ DependecyGraph.prototype.resolveDependency = function(
return dep;
} else {

// 4. `depModuleId` is a module defined in a package relative to
// `fromModule`.
// `depModuleId` is a module defined in a package relative to `fromModule`.
packageJson = this._lookupPackage(fromModule.path);

if (packageJson == null) {
Expand All @@ -224,12 +248,23 @@ DependecyGraph.prototype.resolveDependency = function(
var dir = path.dirname(fromModule.path);
modulePath = path.join(dir, depModuleId);

if (packageJson.browser && typeof packageJson.browser === 'object') {
var relPath = './' + path.relative(packageJson._root, modulePath);
var tmpModulePath = packageJson.browser[withExtJs(relPath)] ||
packageJson.browser[sansExtJs(relPath)];
if (tmpModulePath) {
modulePath = path.join(packageJson._root, tmpModulePath);
}
}

// JS modules can be required without extensios.
if (this._assetExts.indexOf(extname(modulePath)) === -1) {
modulePath = withExtJs(modulePath);
}

dep = this._graph[modulePath];

// Maybe the dependency is a directory and there is an index.js inside it.
if (dep == null) {
modulePath = path.join(dir, depModuleId, 'index.js');
}
Expand Down

0 comments on commit dc4e8d1

Please sign in to comment.