polymer-bundler - npm Package Compare versions

Comparing version 2.0.1 to 2.0.2

CHANGELOG.md

@@ -8,5 +8,10 @@ # Change Log
 
-<!-- ## Unreleased -->
+## Unreleased
 <!-- Add new, unreleased changes here. -->
 
+## 2.0.2 - 2017-06-02
+- In cases such as the shell merge strategy, imports which are not originally
+  part of the shell are now inserted before other imports which are dependent
+  on them.  Fixes https://github.com/Polymer/polymer-bundler/issues/517.
+
 ## 2.0.1 - 2017-05-19
@@ -13,0 +18,0 @@ - Don't include the `by-polymer-bundler` hidden div in bundled files

lib/ast-utils.d.ts

@@ -45,2 +45,16 @@ import { ASTNode, ParserOptions } from 'parse5';
 /**
+ * Returns true if the nodes are given in order as they appear in the source
+ * code.
+ * TODO(usergenic): Port this to `dom5` and do it with typings for location info
+ * instead of all of these string-based lookups.
+ */
+export declare function inSourceOrder(left: ASTNode, right: ASTNode): boolean;
+/**
+ * Returns true if both nodes have the same line and column according to their
+ * location info.
+ * TODO(usergenic): Port this to `dom5` and do it with typings for location info
+ * instead of all of these string-based lookups.
+ */
+export declare function sameNode(node1: ASTNode, node2: ASTNode): boolean;
+/**
  * Return all sibling nodes following node.
@@ -47,0 +61,0 @@ */

lib/ast-utils.js

@@ -117,2 +117,27 @@ "use strict";
 /**
+ * Returns true if the nodes are given in order as they appear in the source
+ * code.
+ * TODO(usergenic): Port this to `dom5` and do it with typings for location info
+ * instead of all of these string-based lookups.
+ */
+function inSourceOrder(left, right) {
+    const l = left.__location, r = right.__location;
+    return l && r && l['line'] && r['line'] &&
+        (l['line'] < r['line'] ||
+            (l['line'] === r['line'] && l['col'] < r['col']));
+}
+exports.inSourceOrder = inSourceOrder;
+/**
+ * Returns true if both nodes have the same line and column according to their
+ * location info.
+ * TODO(usergenic): Port this to `dom5` and do it with typings for location info
+ * instead of all of these string-based lookups.
+ */
+function sameNode(node1, node2) {
+    const l1 = node1.__location, l2 = node2.__location;
+    return !!(l1 && l2 && l1['line'] && l1['col'] && l1['line'] === l2['line'] &&
+        l1['col'] === l2['col']);
+}
+exports.sameNode = sameNode;
+/**
  * Return all sibling nodes following node.
@@ -119,0 +144,0 @@ */

lib/bundler.d.ts

@@ -60,14 +60,2 @@ import { Analyzer } from 'polymer-analyzer';
     /**
-     * Add HTML Import elements for each file in the bundle.  We append all the
-     * imports in the case any were moved into the bundle by the strategy.
-     * While this will almost always yield duplicate imports, they will be
-     * cleaned up through deduplication during the import phase.
-     */
-    private _appendHtmlImportsForBundle(ast, bundle);
-    /**
-     * Append a <link rel="import" node to `node` with a value of `url` for
-     * the "href" attribute.
-     */
-    private _appendHtmlImport(ast, url);
-    /**
      * Set the hidden div at the appropriate location within the document.  The
@@ -93,2 +81,7 @@ * goal is to place the hidden div at the same place as the first html
     /**
+     * Append a `<link rel="import" ...>` node to `node` with a value of `url`
+     * for the "href" attribute.
+     */
+    private _createHtmlImport(url);
+    /**
      * Given an array of Bundles, remove all files from bundles which are in the
@@ -106,2 +99,9 @@ * "excludes" set.  Remove any bundles which are left empty after excluded
     /**
+     * Add HTML Import elements for each file in the bundle.  Efforts are made to
+     * ensure that imports are injected prior to any eager imports of other
+     * bundles which are known to depend on them, to preserve expectations of
+     * evaluation order.
+     */
+    private _injectHtmlImportsForBundle(document, ast, bundle, bundleManifest);
+    /**
      * Replace html import links in the document with the contents of the
@@ -108,0 +108,0 @@ * imported file, but only once per url.

lib/bundler.js

@@ -130,27 +130,2 @@ "use strict";
     /**
-     * Add HTML Import elements for each file in the bundle.  We append all the
-     * imports in the case any were moved into the bundle by the strategy.
-     * While this will almost always yield duplicate imports, they will be
-     * cleaned up through deduplication during the import phase.
-     */
-    _appendHtmlImportsForBundle(ast, bundle) {
-        for (const importUrl of bundle.bundle.files) {
-            const newUrl = urlUtils.relativeUrl(bundle.url, importUrl);
-            if (!newUrl) {
-                continue;
-            }
-            this._appendHtmlImport(this._findOrCreateHiddenDiv(ast), newUrl);
-        }
-    }
-    /**
-     * Append a <link rel="import" node to `node` with a value of `url` for
-     * the "href" attribute.
-     */
-    _appendHtmlImport(ast, url) {
-        const link = dom5.constructors.element('link');
-        dom5.setAttribute(link, 'rel', 'import');
-        dom5.setAttribute(link, 'href', url);
-        dom5.append(ast, link);
-    }
-    /**
      * Set the hidden div at the appropriate location within the document.  The
@@ -189,3 +164,3 @@ * goal is to place the hidden div at the same place as the first html
             dom5.removeFakeRootElements(ast);
-            this._appendHtmlImportsForBundle(ast, docBundle);
+            this._injectHtmlImportsForBundle(document, ast, docBundle, bundleManifest);
             importUtils.rewriteAstToEmulateBaseTag(ast, document.url, this.rewriteUrlsInTemplates);
@@ -228,2 +203,12 @@ // Re-analyzing the document using the updated ast to refresh the scanned
     /**
+     * Append a `<link rel="import" ...>` node to `node` with a value of `url`
+     * for the "href" attribute.
+     */
+    _createHtmlImport(url) {
+        const link = dom5.constructors.element('link');
+        dom5.setAttribute(link, 'rel', 'import');
+        dom5.setAttribute(link, 'href', url);
+        return link;
+    }
+    /**
      * Given an array of Bundles, remove all files from bundles which are in the
@@ -268,2 +253,61 @@ * "excludes" set.  Remove any bundles which are left empty after excluded
     /**
+     * Add HTML Import elements for each file in the bundle.  Efforts are made to
+     * ensure that imports are injected prior to any eager imports of other
+     * bundles which are known to depend on them, to preserve expectations of
+     * evaluation order.
+     */
+    _injectHtmlImportsForBundle(document, ast, bundle, bundleManifest) {
+        // Gather all the document's direct html imports.  We want the direct (not
+        // transitive) imports only here, because we'll be using their AST nodes as
+        // targets to prepended injected imports to.
+        const existingImports = [...document.getFeatures({ kind: 'html-import', noLazyImports: true, imported: false })];
+        const existingImportDependencies = new Map(existingImports.map((existingImport) => [existingImport.document.url, [
+                ...existingImport.document.getFeatures({ kind: 'html-import', imported: true, noLazyImports: true })
+            ].map((feature) => feature.document.url)]));
+        // Every file in the bundle is a candidate for injection into the document.
+        for (const importUrl of bundle.bundle.files) {
+            const relativeImportUrl = urlUtils.relativeUrl(bundle.url, importUrl);
+            // If relative URL is empty, it is because the import URL and the document
+            // URL are the same, so there is no need to inject.
+            if (!relativeImportUrl) {
+                continue;
+            }
+            // If there is an existing import in the document that matches the import
+            // URL already, we don't need to inject one.
+            if (existingImports.find((e) => e.document.url === importUrl)) {
+                continue;
+            }
+            // We are looking for the earliest eager import of an html document which
+            // has a dependency on the html import we want to inject.
+            let prependTarget = undefined;
+            // We are only concerned with imports that are not of files in this
+            // bundle.
+            for (const existingImport of existingImports.filter((e) => !bundle.bundle.files.has(e.document.url))) {
+                // If the existing import has a dependency on the import we are about
+                // to inject, it may be our new target.
+                if (existingImportDependencies.get(existingImport.document.url)
+                    .indexOf(importUrl) !== -1) {
+                    const newPrependTarget = dom5.query(ast, (node) => astUtils.sameNode(node, existingImport.astNode));
+                    // IF we don't have a target already or if the old target comes after
+                    // the new one in the source code, the new one will replace the old
+                    // one.
+                    if (newPrependTarget &&
+                        (!prependTarget ||
+                            astUtils.inSourceOrder(newPrependTarget, prependTarget))) {
+                        prependTarget = newPrependTarget;
+                    }
+                }
+            }
+            // Inject the new html import into the document.
+            const newHtmlImport = this._createHtmlImport(relativeImportUrl);
+            if (prependTarget) {
+                dom5.insertBefore(prependTarget.parentNode, prependTarget, newHtmlImport);
+            }
+            else {
+                const hiddenDiv = this._findOrCreateHiddenDiv(ast);
+                dom5.append(hiddenDiv.parentNode, newHtmlImport);
+            }
+        }
+    }
+    /**
      * Replace html import links in the document with the contents of the
@@ -270,0 +314,0 @@ * imported file, but only once per url.

lib/constants.d.ts

@@ -1,2 +0,2 @@
-declare var _default: {
+declare const _default: {
     EXTERNAL_URL: RegExp;
@@ -3,0 +3,0 @@ ABS_URL: RegExp;

lib/test/ast-utils_test.js

@@ -20,2 +20,4 @@ "use strict";
 const chai = require("chai");
+const clone = require("clone");
+const dom5 = require("dom5");
 const parse5 = require("parse5");
@@ -25,2 +27,15 @@ const ast = require("../ast-utils");
 suite('AST Utils', function () {
+    test('inSourceOrder', () => {
+        const html = parse5.parseFragment(`
+          <span>oh</span><span>hi</span>
+          <span>good</span>
+        <span>bye</span>
+      `, { locationInfo: true });
+        const spans = dom5.queryAll(html, dom5.predicates.hasTagName('span'));
+        assert.isTrue(ast.inSourceOrder(spans[0], spans[1]), 'oh -> hi');
+        assert.isTrue(ast.inSourceOrder(spans[0], spans[3]), 'oh -> bye');
+        assert.isTrue(ast.inSourceOrder(spans[2], spans[3]), 'good -> bye');
+        assert.isFalse(ast.inSourceOrder(spans[3], spans[1]), 'bye <- hi');
+        assert.isFalse(ast.inSourceOrder(spans[1], spans[0]), 'hi <- oh');
+    });
     test('prepend', () => {
@@ -33,2 +48,12 @@ const orderedList = parse5.parseFragment(`<ol><li>1<li>2<li>3<li>4<li>5</ol>`);
     });
+    test('sameNode', () => {
+        const html = parse5.parseFragment(`<div><h1>hi</h1><h1>h1</h1></div>`, { locationInfo: true });
+        const h1_1 = html.childNodes[0].childNodes[0];
+        const h1_2 = html.childNodes[0].childNodes[1];
+        const h1_1_clone = clone(h1_1);
+        assert.isFalse(h1_1 === h1_2);
+        assert.isFalse(h1_1 === h1_1_clone);
+        assert.isFalse(ast.sameNode(h1_1, h1_2));
+        assert.isTrue(ast.sameNode(h1_1, h1_1_clone));
+    });
     test('siblingsAfter', () => {
@@ -35,0 +60,0 @@ const orderedList = parse5.parseFragment(`<ol><li>1<li>2<li>3<li>4<li>5</ol>`);

lib/test/bundler_test.js

@@ -80,3 +80,4 @@ "use strict";
                 inlineScripts: true,
-                // This strategy adds a file not in the original document to the bundle.
+                // This strategy adds a file not in the original document to the
+                // bundle.
                 strategy: (bundles) => {
@@ -566,2 +567,22 @@ bundles.forEach((b) => {
         }));
+        test('eagerly importing a fragment', () => __awaiter(this, void 0, void 0, function* () {
+            const bundler = new bundler_1.Bundler({
+                analyzer: new polymer_analyzer_1.Analyzer({ urlLoader: new polymer_analyzer_1.FSUrlLoader('test/html/imports') }),
+                strategy: bundle_manifest_1.generateShellMergeStrategy('importing-fragments/shell.html'),
+            });
+            const manifest = yield bundler.generateManifest([
+                'eagerly-importing-a-fragment.html',
+                'importing-fragments/fragment-a.html',
+                'importing-fragments/fragment-b.html',
+                'importing-fragments/shell.html',
+            ]);
+            const result = yield bundler.bundle(manifest);
+            assert.equal(result.manifest.bundles.size, 4);
+            const shell = parse5.serialize(result.documents.get('importing-fragments/shell.html').ast);
+            const fragmentAAt = shell.indexOf('href="fragment-a.html"');
+            const shellAt = shell.indexOf(`console.log('shell.html')`);
+            const sharedUtilAt = shell.indexOf(`console.log('shared-util.html')`);
+            assert.isTrue(sharedUtilAt < fragmentAAt, 'Inlined shared-util.html should come before fragment-a.html import');
+            assert.isTrue(fragmentAAt < shellAt, 'fragment-a.html import should come before script in shell.html');
+        }));
         test.skip('Imports in templates should not inline', () => __awaiter(this, void 0, void 0, function* () {
@@ -568,0 +589,0 @@ const doc = yield bundle('test/html/inside-template.html');

lib/test/deps-index_test.js

@@ -89,4 +89,20 @@ "use strict";
         }));
+        test('when an entrypoint imports an entrypoint', () => __awaiter(this, void 0, void 0, function* () {
+            const entrypoint = 'eagerly-importing-a-fragment.html';
+            const fragmentA = 'importing-fragments/fragment-a.html';
+            const fragmentB = 'importing-fragments/fragment-a.html';
+            const util = 'importing-fragments/shared-util.html';
+            const shell = 'importing-fragments/shell.html';
+            const analyzer = new polymer_analyzer_1.Analyzer({ urlLoader: new polymer_analyzer_1.FSUrlLoader('test/html/imports') });
+            const expectedEntrypointsToDeps = new Map([
+                [entrypoint, new Set([entrypoint, fragmentA, shell, util])],
+                [fragmentA, new Set([fragmentA, util])],
+                [fragmentB, new Set([fragmentB, util])],
+                [shell, new Set([shell, fragmentA, util])],
+            ]);
+            const index = yield deps_index_1.buildDepsIndex([entrypoint, fragmentA, fragmentB, shell], analyzer);
+            chai.assert.deepEqual(serializeMap(index.entrypointToDeps), serializeMap(expectedEntrypointsToDeps));
+        }));
     });
 });
 //# sourceMappingURL=deps-index_test.js.map

package.json

 {
   "name": "polymer-bundler",
-  "version": "2.0.1",
+  "version": "2.0.2",
   "description": "Process Web Components into one output file",
@@ -20,8 +20,5 @@ "main": "lib/bundler.js",
     "dom5": "^2.2.0",
-    "es6-promise": "^2.1.0",
     "espree": "^3.4.0",
     "mkdirp": "^0.5.1",
-    "nopt": "^3.0.1",
     "parse5": "^2.2.2",
-    "path-posix": "^1.0.0",
     "polymer-analyzer": "^2.0.0",
@@ -28,0 +25,0 @@ "source-map": "^0.5.6"

No alert changes

Improved metrics

Total package byte prevSize

366854

increased by3.98%

Dependency count

9

decreased by-25%

Lines of code

4378

increased by3.43%

Dependency changes

• - Removedes6-promise@^2.1.0

• - Removednopt@^3.0.1

• - Removedpath-posix@^1.0.0

• - Removedabbrev@1.1.1(transitive)

• - Removedes6-promise@2.3.0(transitive)

• - Removednopt@3.0.6(transitive)

• - Removedpath-posix@1.0.0(transitive)