Fix push manifest generation bugs affecting bower_components and ES m…

…odules (#512) * Add failing test for push manifest generation with bower_components. * Use paths relative to project root instead of package root in push manifests. * Skip bundler's fake sub-bundle URLs when generating push manifests. * Add failing test for push manifest generation with ES modules. * Use an FsUrlResolver with the project root for push manifest generation. Previously it looks like we were implicitly relying on the current working directory? This also means we don't have to manually fix up relative URLs in push manifest generation, so reverted that. * Update CHANGELOGs for push manifest fixes.
Polymer · Jun 16, 2018 · ec5693d · ec5693d
1 parent 4fb8d89
commit ec5693d
Show file tree

Hide file tree

Showing 12 changed files with 80 additions and 14 deletions.
diff --git a/packages/build/CHANGELOG.md b/packages/build/CHANGELOG.md
@@ -5,7 +5,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-<!-- ## Unreleased -->
+## Unreleased
+* Fix incorrect relative paths to the component directory in push manifests.
+* Fix push manifest generation crash with ES module projects.
 <!-- Add new, unreleased changes here. -->
 
 ## [3.0.1] - 2018-05-14

diff --git a/packages/build/src/push-manifest.ts b/packages/build/src/push-manifest.ts
@@ -13,7 +13,7 @@
  */
 
 import * as path from 'path';
-import {Analyzer, Import, PackageRelativeUrl, ResolvedUrl} from 'polymer-analyzer';
+import {Analyzer, FsUrlResolver, Import, PackageRelativeUrl, ResolvedUrl} from 'polymer-analyzer';
 import {buildDepsIndex} from 'polymer-bundler/lib/deps-index';
 import {ProjectConfig} from 'polymer-project-config';
 
@@ -181,7 +181,10 @@ export class AddPushManifest extends AsyncTransformStream<File, File> {
     super({objectMode: true});
     this.files = new Map();
     this.config = config;
-    this.analyzer = new Analyzer({urlLoader: new FileMapUrlLoader(this.files)});
+    this.analyzer = new Analyzer({
+      urlLoader: new FileMapUrlLoader(this.files),
+      urlResolver: new FsUrlResolver(config.root),
+    });
     this.outPath =
         path.join(this.config.root, outPath || 'push-manifest.json') as
         LocalFsPath;
@@ -212,13 +215,15 @@ export class AddPushManifest extends AsyncTransformStream<File, File> {
     // Bundler's buildDepsIndex code generates an index with all fragments and
     // all lazy-imports encountered are the keys, so we'll use that function to
     // produce the set of all fragments to generate push-manifest entries for.
-    const allFragments = new Set(
-        (await buildDepsIndex(
-             this.config.allFragments.map(
-                 (path) => this.analyzer.resolveUrl(urlFromPath(
-                     this.config.root as LocalFsPath, path as LocalFsPath))!),
-             this.analyzer))
-            .keys());
+    const depsIndex = await buildDepsIndex(
+        this.config.allFragments.map(
+            (path) => this.analyzer.resolveUrl(urlFromPath(
+                this.config.root as LocalFsPath, path as LocalFsPath))!),
+        this.analyzer);
+    // Don't include bundler's fake "sub-bundle" URLs (e.g.
+    // "foo.html>external#1>bar.js").
+    const allFragments =
+        new Set([...depsIndex.keys()].filter((url) => !url.includes('>')));
 
     // If an app-shell exists, use that as our main push URL because it has a
     // reliable URL. Otherwise, support the single entrypoint URL.

diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md
@@ -1,7 +1,8 @@
 # Changelog
 
-
-<!-- ## Unreleased -->
+## Unreleased
+* Fix incorrect relative paths to the component directory in push manifests.
+* Fix push manifest generation crash with ES module projects.
 <!-- Add new, unreleased items here. -->
 
 ## v1.7.3 [06-11-2018]

diff --git a/packages/cli/src/test/integration/build_test.ts b/packages/cli/src/test/integration/build_test.ts
@@ -139,6 +139,18 @@ suite('polymer build', function() {
         path.join(fixtureDir, 'polymer-2-project', 'expected/default'));
   });
 
+  test('handles polymer 2.x push manifest', async () => {
+    const tmpDir = tmp.dirSync();
+    copyDir(path.join(fixtureDir, 'polymer-2-project', 'source'), tmpDir.name);
+
+    await runCommand(binPath, ['build', '--name=push', '--add-push-manifest'], {
+      cwd: tmpDir.name
+    });
+    assertDirsEqual(
+        path.join(tmpDir.name, 'build/push'),
+        path.join(fixtureDir, 'polymer-2-project', 'expected/push'));
+  });
+
   test('handles bundle tagged-template literals in ES5', async () => {
     const tmpDir = tmp.dirSync();
     copyDir(

diff --git a/packages/cli/test/fixtures/build-modules/expected/polymer.json b/packages/cli/test/fixtures/build-modules/expected/polymer.json
@@ -11,7 +11,8 @@
   "builds": [
     {
       "name": "unbundled-es",
-      "bundle": false
+      "bundle": false,
+      "addPushManifest": true
     },
     {
       "name": "unbundled-amd",

diff --git a/packages/cli/test/fixtures/build-modules/expected/unbundled-es/push-manifest.json b/packages/cli/test/fixtures/build-modules/expected/unbundled-es/push-manifest.json
@@ -0,0 +1,16 @@
+{
+  "index.html": {
+    "mod1.js": {
+      "type": "script",
+      "weight": 1
+    },
+    "mod2.js": {
+      "type": "script",
+      "weight": 1
+    },
+    "node_modules/dep1/dep1.js": {
+      "type": "script",
+      "weight": 1
+    }
+  }
+}
diff --git a/packages/cli/test/fixtures/build-modules/source/polymer.json b/packages/cli/test/fixtures/build-modules/source/polymer.json
@@ -4,7 +4,8 @@
   "builds": [
     {
       "name": "unbundled-es",
-      "bundle": false
+      "bundle": false,
+      "addPushManifest": true
     },
     {
       "name": "unbundled-amd",

diff --git a/...s/cli/test/fixtures/polymer-2-project/expected/push/bower_components/polymer/polymer.html b/...s/cli/test/fixtures/polymer-2-project/expected/push/bower_components/polymer/polymer.html
@@ -0,0 +1 @@
+<!-- polymer code goes here -->
diff --git a/packages/cli/test/fixtures/polymer-2-project/expected/push/index.html b/packages/cli/test/fixtures/polymer-2-project/expected/push/index.html
@@ -0,0 +1 @@
+<link rel="import" href="/src/my-app.html">
diff --git a/packages/cli/test/fixtures/polymer-2-project/expected/push/push-manifest.json b/packages/cli/test/fixtures/polymer-2-project/expected/push/push-manifest.json
@@ -0,0 +1,16 @@
+{
+  "index.html": {
+    "src/my-app.html": {
+      "type": "document",
+      "weight": 1
+    },
+    "bower_components/polymer/polymer.html": {
+      "type": "document",
+      "weight": 1
+    },
+    "src/elements/my-element.html": {
+      "type": "document",
+      "weight": 1
+    }
+  }
+}
diff --git a/packages/cli/test/fixtures/polymer-2-project/expected/push/src/elements/my-element.html b/packages/cli/test/fixtures/polymer-2-project/expected/push/src/elements/my-element.html
@@ -0,0 +1,7 @@
+<dom-module id="my-element">
+  <template>
+    <style>
+      :host { background-image: url('bg.png'); }
+    </style>
+  </template>
+</dom-module>
diff --git a/packages/cli/test/fixtures/polymer-2-project/expected/push/src/my-app.html b/packages/cli/test/fixtures/polymer-2-project/expected/push/src/my-app.html
@@ -0,0 +1,3 @@
+<link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="elements/my-element.html">
+<my-element></my-element>