From 97d03544e2bf077201a887124399f2970782365f Mon Sep 17 00:00:00 2001 From: ranfdev Date: Mon, 18 Jun 2018 13:49:19 +0200 Subject: [PATCH 1/7] get only existing package main --- src/Resolver.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/Resolver.js b/src/Resolver.js index ab96d494ee7..674a9cee0d7 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -229,7 +229,7 @@ class Resolver { pkg = await this.readPackage(dir); // First try loading package.main as a file, then try as a directory. - let main = this.getPackageMain(pkg); + let main = await this.getPackageMain(pkg); let res = (await this.loadAsFile(main, extensions, pkg)) || (await this.loadDirectory(main, extensions, pkg)); @@ -270,10 +270,8 @@ class Resolver { return pkg; } - getBrowserField(pkg) { - let target = this.options.target || 'browser'; - return target === 'browser' ? pkg.browser : null; - } + async getPackageMain(pkg) { + let {browser} = pkg; getPackageMain(pkg) { let browser = this.getBrowserField(pkg); @@ -284,16 +282,14 @@ class Resolver { // libraries like d3.js specifies node.js specific files in the "main" which breaks the build // we use the "browser" or "module" field to get the full dependency tree if available. // If this is a linked module with a `source` field, use that as the entry point. - let main = [pkg.source, browser, pkg.module, pkg.main].find( - entry => typeof entry === 'string' - ); - - // Default to index file if no main field find - if (!main || main === '.' || main === './') { - main = 'index'; + for (let source of [pkg.source, pkg.module, browser, pkg.main, 'index']) { + if ( + typeof source === 'string' && + (await fs.exists(path.resolve(pkg.pkgdir, source))) + ) { + return path.resolve(pkg.pkgdir, source); + } } - - return path.resolve(pkg.pkgdir, main); } async loadAsFile(file, extensions, pkg) { From 8c49510c1d9b7b1e35f6103cba7bbfe02e5ac524 Mon Sep 17 00:00:00 2001 From: ranfdev Date: Tue, 19 Jun 2018 14:39:23 +0200 Subject: [PATCH 2/7] rewrite of getPackageMain, better fallback in case no main found --- src/Resolver.js | 35 +++++++++++++++++++++++++++++------ src/utils/fs.js | 12 ++---------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/Resolver.js b/src/Resolver.js index 674a9cee0d7..0e11102f06c 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -282,14 +282,37 @@ class Resolver { // libraries like d3.js specifies node.js specific files in the "main" which breaks the build // we use the "browser" or "module" field to get the full dependency tree if available. // If this is a linked module with a `source` field, use that as the entry point. - for (let source of [pkg.source, pkg.module, browser, pkg.main, 'index']) { - if ( - typeof source === 'string' && - (await fs.exists(path.resolve(pkg.pkgdir, source))) - ) { - return path.resolve(pkg.pkgdir, source); + let main; + // this is a list possibile main files + let possibleMain = [pkg.main, browser, pkg.module, pkg.source]; + // now we iter trough the list, from the end to the beginning. + // by doing this, we can easily pop elements from the list + for (let i = possibleMain.length - 1; i >= 0; i--) { + main = possibleMain[i]; + if (typeof main !== 'string') { + // if the current main is not a string, trash it + possibleMain.pop(); + continue; } + try { + // else, check if the file main exists/is accessible. if it does, break the loop. + await fs.access(path.resolve(pkg.pkgdir, main)); + break; + } catch (e) { + // keep looking for an accessible file + } + } + + // if after the loop we didnt't find any existing file, assign + // to the main variable the first string we found. + // this is needed in certain cases. + // example: 'should use the source field as a glob alias when symlinked' in test/resolver.js + main = main || possibleMain.pop(); + // fallback + if (!main || main === '.' || main === './') { + main = 'index'; } + return path.resolve(pkg.pkgdir, main); } async loadAsFile(file, extensions, pkg) { diff --git a/src/utils/fs.js b/src/utils/fs.js index 830efa410aa..b2f83d2a5a3 100644 --- a/src/utils/fs.js +++ b/src/utils/fs.js @@ -7,16 +7,8 @@ exports.writeFile = promisify(fs.writeFile); exports.stat = promisify(fs.stat); exports.readdir = promisify(fs.readdir); exports.unlink = promisify(fs.unlink); -exports.realpath = async function(path) { - const realpath = promisify(fs.realpath); - try { - path = await realpath(path); - } catch (e) { - // do nothing - } - return path; -}; -exports.lstat = promisify(fs.lstat); +exports.realpath = promisify(fs.realpath); +exports.access = promisify(fs.access); exports.exists = function(filename) { return new Promise(resolve => { From 45d4a62681bdac08022473480efc256af364821f Mon Sep 17 00:00:00 2001 From: ranfdev Date: Tue, 19 Jun 2018 14:57:59 +0200 Subject: [PATCH 3/7] little optimizations to getPackageMain --- src/Resolver.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Resolver.js b/src/Resolver.js index 0e11102f06c..4283c6a42ad 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -295,9 +295,10 @@ class Resolver { continue; } try { - // else, check if the file main exists/is accessible. if it does, break the loop. - await fs.access(path.resolve(pkg.pkgdir, main)); - break; + // else, check if the file main exists/is accessible. if it does, return the path of the file. + const resolvedPath = path.resolve(pkg.pkgdir, main); + await fs.access(resolvedPath); + return resolvedPath; } catch (e) { // keep looking for an accessible file } From e0177bf7be7eb940d4c4a5298c039037a29ce646 Mon Sep 17 00:00:00 2001 From: ranfdev Date: Thu, 12 Jul 2018 12:41:30 +0200 Subject: [PATCH 4/7] moved getPackageMain logic to loadDirectory --- src/Resolver.js | 60 ++++++++++++++++--------------------------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/src/Resolver.js b/src/Resolver.js index 4283c6a42ad..db4d6851f35 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -228,14 +228,23 @@ class Resolver { try { pkg = await this.readPackage(dir); - // First try loading package.main as a file, then try as a directory. - let main = await this.getPackageMain(pkg); - let res = - (await this.loadAsFile(main, extensions, pkg)) || - (await this.loadDirectory(main, extensions, pkg)); - - if (res) { - return res; + let fileList = this.listPossibleFiles(pkg); + for (let file of fileList) { + if (typeof file != 'string') { + continue; + } + if (file == '.' || file == './') { + file = 'index'; + } else { + file = path.resolve(pkg.pkgdir, file); + } + + const res = + (await this.loadAsFile(file, extensions, pkg)) || + (await this.loadDirectory(file, extensions, pkg)); + if (res) { + return res; + } } } catch (err) { // ignore @@ -270,7 +279,7 @@ class Resolver { return pkg; } - async getPackageMain(pkg) { + listPossibleFiles(pkg) { let {browser} = pkg; getPackageMain(pkg) { @@ -282,38 +291,7 @@ class Resolver { // libraries like d3.js specifies node.js specific files in the "main" which breaks the build // we use the "browser" or "module" field to get the full dependency tree if available. // If this is a linked module with a `source` field, use that as the entry point. - let main; - // this is a list possibile main files - let possibleMain = [pkg.main, browser, pkg.module, pkg.source]; - // now we iter trough the list, from the end to the beginning. - // by doing this, we can easily pop elements from the list - for (let i = possibleMain.length - 1; i >= 0; i--) { - main = possibleMain[i]; - if (typeof main !== 'string') { - // if the current main is not a string, trash it - possibleMain.pop(); - continue; - } - try { - // else, check if the file main exists/is accessible. if it does, return the path of the file. - const resolvedPath = path.resolve(pkg.pkgdir, main); - await fs.access(resolvedPath); - return resolvedPath; - } catch (e) { - // keep looking for an accessible file - } - } - - // if after the loop we didnt't find any existing file, assign - // to the main variable the first string we found. - // this is needed in certain cases. - // example: 'should use the source field as a glob alias when symlinked' in test/resolver.js - main = main || possibleMain.pop(); - // fallback - if (!main || main === '.' || main === './') { - main = 'index'; - } - return path.resolve(pkg.pkgdir, main); + return [pkg.source, browser, pkg.module, pkg.main]; } async loadAsFile(file, extensions, pkg) { From 21c9b96f6bca85133e1c5cfbd6ce1ec1fe614118 Mon Sep 17 00:00:00 2001 From: ranfdev Date: Thu, 12 Jul 2018 12:55:32 +0200 Subject: [PATCH 5/7] resolve index file --- src/Resolver.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Resolver.js b/src/Resolver.js index db4d6851f35..0973503098b 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -235,9 +235,8 @@ class Resolver { } if (file == '.' || file == './') { file = 'index'; - } else { - file = path.resolve(pkg.pkgdir, file); } + file = path.resolve(pkg.pkgdir, file); const res = (await this.loadAsFile(file, extensions, pkg)) || From 49f7ad0e3427f7cc78f0b18a75265248b7c6d1ac Mon Sep 17 00:00:00 2001 From: ranfdev Date: Thu, 12 Jul 2018 13:01:01 +0200 Subject: [PATCH 6/7] reverted useless change to fs.js --- src/utils/fs.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/utils/fs.js b/src/utils/fs.js index b2f83d2a5a3..830efa410aa 100644 --- a/src/utils/fs.js +++ b/src/utils/fs.js @@ -7,8 +7,16 @@ exports.writeFile = promisify(fs.writeFile); exports.stat = promisify(fs.stat); exports.readdir = promisify(fs.readdir); exports.unlink = promisify(fs.unlink); -exports.realpath = promisify(fs.realpath); -exports.access = promisify(fs.access); +exports.realpath = async function(path) { + const realpath = promisify(fs.realpath); + try { + path = await realpath(path); + } catch (e) { + // do nothing + } + return path; +}; +exports.lstat = promisify(fs.lstat); exports.exists = function(filename) { return new Promise(resolve => { From 2daf2e259ac06afe0dbc9152df6e097e55156ec7 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sat, 21 Jul 2018 15:25:40 -0700 Subject: [PATCH 7/7] Cleanup + add tests --- src/Resolver.js | 32 +++++++++++-------- .../package-module-fallback/main.js | 0 .../package-module-fallback/package.json | 5 +++ test/resolver.js | 12 +++++++ 4 files changed, 36 insertions(+), 13 deletions(-) create mode 100644 test/integration/resolver/node_modules/package-module-fallback/main.js create mode 100644 test/integration/resolver/node_modules/package-module-fallback/package.json diff --git a/src/Resolver.js b/src/Resolver.js index 0973503098b..9f00f52dd18 100755 --- a/src/Resolver.js +++ b/src/Resolver.js @@ -228,16 +228,11 @@ class Resolver { try { pkg = await this.readPackage(dir); - let fileList = this.listPossibleFiles(pkg); - for (let file of fileList) { - if (typeof file != 'string') { - continue; - } - if (file == '.' || file == './') { - file = 'index'; - } - file = path.resolve(pkg.pkgdir, file); + // Get a list of possible package entry points. + let entries = this.getPackageEntries(pkg); + for (let file of entries) { + // First try loading package.main as a file, then try as a directory. const res = (await this.loadAsFile(file, extensions, pkg)) || (await this.loadDirectory(file, extensions, pkg)); @@ -278,10 +273,12 @@ class Resolver { return pkg; } - listPossibleFiles(pkg) { - let {browser} = pkg; + getBrowserField(pkg) { + let target = this.options.target || 'browser'; + return target === 'browser' ? pkg.browser : null; + } - getPackageMain(pkg) { + getPackageEntries(pkg) { let browser = this.getBrowserField(pkg); if (browser && typeof browser === 'object' && browser[pkg.name]) { browser = browser[pkg.name]; @@ -290,7 +287,16 @@ class Resolver { // libraries like d3.js specifies node.js specific files in the "main" which breaks the build // we use the "browser" or "module" field to get the full dependency tree if available. // If this is a linked module with a `source` field, use that as the entry point. - return [pkg.source, browser, pkg.module, pkg.main]; + return [pkg.source, browser, pkg.module, pkg.main] + .filter(entry => typeof entry === 'string') + .map(main => { + // Default to index file if no main field find + if (!main || main === '.' || main === './') { + main = 'index'; + } + + return path.resolve(pkg.pkgdir, main); + }); } async loadAsFile(file, extensions, pkg) { diff --git a/test/integration/resolver/node_modules/package-module-fallback/main.js b/test/integration/resolver/node_modules/package-module-fallback/main.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/integration/resolver/node_modules/package-module-fallback/package.json b/test/integration/resolver/node_modules/package-module-fallback/package.json new file mode 100644 index 00000000000..e64cf59ee7e --- /dev/null +++ b/test/integration/resolver/node_modules/package-module-fallback/package.json @@ -0,0 +1,5 @@ +{ + "name": "package-module-fallback", + "main": "main.js", + "module": "module.js" +} diff --git a/test/resolver.js b/test/resolver.js index ed1fc739569..ff440a0bf89 100644 --- a/test/resolver.js +++ b/test/resolver.js @@ -176,6 +176,18 @@ describe('resolver', function() { assert.equal(resolved.pkg.name, 'package-browser'); }); + it('should fall back to package.main when package.module does not exist', async function() { + let resolved = await resolver.resolve( + 'package-module-fallback', + path.join(rootDir, 'foo.js') + ); + assert.equal( + resolved.path, + path.join(rootDir, 'node_modules', 'package-module-fallback', 'main.js') + ); + assert.equal(resolved.pkg.name, 'package-module-fallback'); + }); + it('should not resolve a node_modules package.browser main field with --target=node', async function() { let resolver = new Resolver({ rootDir,