From d2f82b1dfd0dbfe526955d33150e607850f9fced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Bigio?= Date: Fri, 21 Aug 2015 06:51:18 -0700 Subject: [PATCH] [react-packager] Move async dependencies to `System.import` Summary: We've decided to move the syntax for asynchronously requiring async dependencies. The new syntax works better with promises and therefore withe async/await as well. The new syntax looks like this: `System.import('moduleA').then(moduleA => {...});` or if you're using async/await you could simply do: let moduleA = await System.import('moduleA'); new moduleA().someFunction(); If you need to require multiple dependencies just do: Promise .all([System.import('moduleA'), System.import('moduleB')]) .then((moduleA, moduleB) => {...}) or the equivalent using async/await --- .../BundlesLayoutIntegration-test.js | 106 +++--------------- .../src/DependencyResolver/Module.js | 42 +------ .../__tests__/Module-test.js | 48 ++------ .../src/DependencyResolver/replacePatterns.js | 3 +- 4 files changed, 32 insertions(+), 167 deletions(-) diff --git a/packager/react-packager/src/BundlesLayout/__tests__/BundlesLayoutIntegration-test.js b/packager/react-packager/src/BundlesLayout/__tests__/BundlesLayoutIntegration-test.js index 9ff0f4354..e1db98ab4 100644 --- a/packager/react-packager/src/BundlesLayout/__tests__/BundlesLayoutIntegration-test.js +++ b/packager/react-packager/src/BundlesLayout/__tests__/BundlesLayoutIntegration-test.js @@ -169,7 +169,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]);`, + System.import("a");`, 'a.js': ` /**, * @providesModule a @@ -199,8 +199,8 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]); - require.ensure(["b"]);`, + System.import("a"); + System.import("b");`, 'a.js': ` /**, * @providesModule a @@ -233,40 +233,6 @@ describe('BundlesLayout', () => { ); }); - pit('should put related async dependencies into the same bundle', () => { - setMockFilesystem({ - 'root': { - 'index.js': ` - /** - * @providesModule index - */ - require.ensure(["a", "b"]);`, - 'a.js': ` - /**, - * @providesModule a - */`, - 'b.js': ` - /** - * @providesModule b - */`, - } - }); - - return newBundlesLayout().generateLayout(['/root/index.js']).then(bundles => - stripPolyfills(bundles).then(resolvedBundles => - expect(resolvedBundles).toEqual({ - id: 'bundle.0', - modules: ['/root/index.js'], - children: [{ - id: 'bundle.0.1', - modules: ['/root/a.js', '/root/b.js'], - children: [], - }], - }) - ) - ); - }); - pit('should fully traverse sync dependencies', () => { setMockFilesystem({ 'root': { @@ -275,7 +241,7 @@ describe('BundlesLayout', () => { * @providesModule index */ require("a"); - require.ensure(["b"]);`, + System.import("b");`, 'a.js': ` /**, * @providesModule a @@ -309,7 +275,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]);`, + System.import("a");`, 'a.js': ` /**, * @providesModule a @@ -349,8 +315,8 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]); - require.ensure(["b"]);`, + System.import("a"); + System.import("b");`, 'a.js': ` /**, * @providesModule a @@ -397,12 +363,12 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]);`, + System.import("a");`, 'a.js': ` /**, * @providesModule a */, - require.ensure(["b"]);`, + System.import("b");`, 'b.js': ` /** * @providesModule b @@ -436,46 +402,6 @@ describe('BundlesLayout', () => { ); }); - pit('should dedup same async bundle duplicated dependencies', () => { - setMockFilesystem({ - 'root': { - 'index.js': ` - /** - * @providesModule index - */ - require.ensure(["a", "b"]);`, - 'a.js': ` - /**, - * @providesModule a - */, - require("c");`, - 'b.js': ` - /** - * @providesModule b - */ - require("c");`, - 'c.js': ` - /** - * @providesModule c - */`, - } - }); - - return newBundlesLayout().generateLayout(['/root/index.js']).then(bundles => - stripPolyfills(bundles).then(resolvedBundles => - expect(resolvedBundles).toEqual({ - id: 'bundle.0', - modules: ['/root/index.js'], - children: [{ - id: 'bundle.0.1', - modules: ['/root/a.js', '/root/c.js', '/root/b.js'], - children: [], - }], - }) - ) - ); - }); - pit('should put image dependencies into separate bundles', () => { setMockFilesystem({ 'root': { @@ -483,7 +409,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]);`, + System.import("a");`, 'a.js':` /**, * @providesModule a @@ -515,8 +441,8 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]); - require.ensure(["b"]);`, + System.import("a"); + System.import("b");`, 'a.js':` /**, * @providesModule a @@ -560,7 +486,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["./img.png"]);`, + System.import("./img.png");`, 'img.png': '', } }); @@ -587,7 +513,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["a"]);`, + System.import("a");`, 'a.js':` /**, * @providesModule a @@ -619,7 +545,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["image!img"]);`, + System.import("image!img");`, 'img.png': '', } }); @@ -646,7 +572,7 @@ describe('BundlesLayout', () => { /** * @providesModule index */ - require.ensure(["aPackage"]);`, + System.import("aPackage");`, 'aPackage': { 'package.json': JSON.stringify({ name: 'aPackage', diff --git a/packager/react-packager/src/DependencyResolver/Module.js b/packager/react-packager/src/DependencyResolver/Module.js index f7ae802a8..72584f8f9 100644 --- a/packager/react-packager/src/DependencyResolver/Module.js +++ b/packager/react-packager/src/DependencyResolver/Module.js @@ -135,9 +135,6 @@ class Module { */ const blockCommentRe = /\/\*(.|\n)*?\*\//g; const lineCommentRe = /\/\/.+(\n|$)/g; -const trailingCommaRe = /,\s*$/g; -const removeSpacesRe = /\s/g; -const quotesRe = /'/g; function extractRequires(code /*: string*/) /*: Array*/ { var deps = { sync: [], @@ -163,40 +160,11 @@ function extractRequires(code /*: string*/) /*: Array*/ { // Parse async dependencies this module has. As opposed to what happens // with sync dependencies, when the module is required, it's async // dependencies won't be loaded into memory. This is deferred till the - // code path gets to a `require.ensure` statement. The syntax is similar - // to webpack's one: - // require.ensure(['dep1', 'dep2'], () => { - // var dep1 = require('dep1'); - // var dep2 = require('dep2'); - // // do something with dep1 and dep2 - // }); - .replace(replacePatterns.REQUIRE_ENSURE_RE, (match, dep, post) => { - dep = dep - .replace(blockCommentRe, '') - .replace(lineCommentRe, '') - .replace(trailingCommaRe, '') - .replace(removeSpacesRe, '') - .replace(quotesRe, '"'); - - if (dep) { - try { - dep = JSON.parse('[' + dep + ']'); - } catch(e) { - throw 'Error processing `require.ensure` while attemping to parse ' + - 'dependencies `[' + dep + ']`: ' + e; - } - - dep.forEach(d => { - if (typeof d !== 'string') { - throw 'Error processing `require.ensure`: dependencies `[' + - d + ']` must be string literals'; - } - }); - - // TODO: throw error if there are duplicate dependencies - - deps.async.push(dep); - } + // code path gets to the import statement: + // System.import('dep1') + .replace(replacePatterns.SYSTEM_IMPORT_RE, (match, pre, quot, dep, post) => { + deps.async.push([dep]); + return match; }); return deps; diff --git a/packager/react-packager/src/DependencyResolver/__tests__/Module-test.js b/packager/react-packager/src/DependencyResolver/__tests__/Module-test.js index 637e84cbe..171e82307 100644 --- a/packager/react-packager/src/DependencyResolver/__tests__/Module-test.js +++ b/packager/react-packager/src/DependencyResolver/__tests__/Module-test.js @@ -57,7 +57,7 @@ describe('Module', () => { pit('should recognize single dependency', () => { fs.__setMockFilesystem({ 'root': { - 'index.js': 'require.ensure(["dep1"], function() {});', + 'index.js': 'System.import("dep1")', } }); @@ -67,67 +67,37 @@ describe('Module', () => { pit('should parse single quoted dependencies', () => { fs.__setMockFilesystem({ 'root': { - 'index.js': 'require.ensure([\'dep1\'], function() {});', + 'index.js': 'System.import(\'dep1\')', } }); return expectAsyncDependenciesToEqual([['dep1']]); }); - pit('should recognize multiple dependencies on the same statement', () => { - fs.__setMockFilesystem({ - 'root': { - 'index.js': 'require.ensure(["dep1", "dep2"], function() {});', - } - }); - - return expectAsyncDependenciesToEqual([['dep1', 'dep2']]); - }); - - pit('should group async dependencies', () => { + pit('should parse multiple async dependencies on the same module', () => { fs.__setMockFilesystem({ 'root': { 'index.js': [ - 'require.ensure(["dep1", "dep2"], function() {});', - 'require.ensure(["dep3", "dep4"], function() {});', + 'System.import("dep1")', + 'System.import("dep2")', ].join('\n'), } }); return expectAsyncDependenciesToEqual([ - ['dep1', 'dep2'], - ['dep3', 'dep4'] + ['dep1'], + ['dep2'], ]); }); - pit('shouldn\'t throw with ES6 arrow functions', () => { - fs.__setMockFilesystem({ - 'root': { - 'index.js': 'require.ensure(["dep1", "dep2"], () => {});', - } - }); - - return expectAsyncDependenciesToEqual([['dep1', 'dep2']]); - }); - pit('parse fine new lines', () => { fs.__setMockFilesystem({ 'root': { - 'index.js': 'require.ensure(["dep1", \n"dep2"], () => {});', + 'index.js': 'System.import(\n"dep1"\n)', } }); - return expectAsyncDependenciesToEqual([['dep1', 'dep2']]); - }); - - pit('ignore comments', () => { - fs.__setMockFilesystem({ - 'root': { - 'index.js': 'require.ensure(["dep1", /*comment*/"dep2"], () => {});', - } - }); - - return expectAsyncDependenciesToEqual([['dep1', 'dep2']]); + return expectAsyncDependenciesToEqual([['dep1']]); }); }); }); diff --git a/packager/react-packager/src/DependencyResolver/replacePatterns.js b/packager/react-packager/src/DependencyResolver/replacePatterns.js index f683331a6..c27d7c770 100644 --- a/packager/react-packager/src/DependencyResolver/replacePatterns.js +++ b/packager/react-packager/src/DependencyResolver/replacePatterns.js @@ -11,4 +11,5 @@ exports.IMPORT_RE = /(\bimport\s+?(?:.+\s+?from\s+?)?)(['"])([^'"]+)(\2)/g; exports.REQUIRE_RE = /(\brequire\s*?\(\s*?)(['"])([^'"]+)(\2\s*?\))/g; -exports.REQUIRE_ENSURE_RE = /\brequire\.ensure\s*\(\s*(?:\[([^\]]+)\])?/g; +exports.SYSTEM_IMPORT_RE = /(\bSystem\.import\s*?\(\s*?)(['"])([^'"]+)(\2\s*?\))/g; +