New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 921285 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 915053



Sign in to add a comment

Update polymer-bundler to version 4.x

Project Member Reported by dpa...@chromium.org, Jan 12

Issue description

The 4.x version has a the big advantage that it supports Javascript modules, but also it unfortunately requires more transitive dependencies.

 
Blocking: 915053
Cc: brendanb@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0fba7392ac90366b3160e5cfd5c324e1666d078

commit f0fba7392ac90366b3160e5cfd5c324e1666d078
Author: dpapad <dpapad@chromium.org>
Date: Mon Jan 14 20:13:40 2019

WebUI: Don't use relative paths for aliased files in managed-footnote.

This is blocking rolling polymer-bundler to version 4.x, by throwing a build
time error. It is also a bit unclear whether this works correctly with the
current 3.x version.

Bug: 921285
Change-Id: I29c40b177771256f4fa800ec53d9ff0b03b0c7b0
Reviewed-on: https://chromium-review.googlesource.com/c/1407261
Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622567}
[modify] https://crrev.com/f0fba7392ac90366b3160e5cfd5c324e1666d078/ui/webui/resources/cr_components/managed_footnote/managed_footnote.html

Owner: aee@chromium.org
Context:

1) In order to use modules from any chrome:// URL you need to patch the following CL first (curerntly in review). https://chromium-review.googlesource.com/c/chromium/src/+/1405496

2) See https://chromium-review.googlesource.com/c/chromium/src/+/1376794 to understand the big picture first. It is a proof of concept of how we can start using modules and eliminate unnecessary HTML dummy files for non-UI elements. You should be able to run this locally with optimize_webui=false.


Goal of this bug:

The goal of this bug is to make it work with optimize_webui=true as well, which currently throws a build error. The process for getting this to work is roughly as follows:

 a) Test the new version locally. 
 b) Investigate how it breaks, and try to come up with a minimal repro example. You will most likely have to inspect the output files under out/<out_folder>/gchrome/chrome/browser/resources/...
 c) Communicate any findings back to polymer-bundler author, and wait for a fix.
 d) Once a fix exists, go back to Step a. 


Instructions:
 1) Patch https://chromium-review.googlesource.com/c/chromium/src/+/1410383 locally.
 2) In third_party/node, backup the existing node_modules folder.
    mv node_modules/ node_modules_v3;
 3) Execute https://cs.chromium.org/chromium/src/third_party/node/update_npm_deps locally (ignore
    the printed message about uploading the new version to the server for now).
 4) Follow abcd steps above.
 5) Move back and forth between old and new versions with these handy commands
    mv node_modules node_modules_v3;mv node_modules_v4/ node_modules OR
    mv node_modules node_modules_v4;mv node_modules_v3/ node_modules

I hope this makes a little bit of sense. Feel free to grab me in person for questions.



Comment 6 by aee@chromium.org, Jan 18 (5 days ago)

Here is a minimal repro of the issue highlighting the difference between v3 and v4 with the --rewrite-urls-in-templates switch. Basically when a module is being brought into the bundle, the relative paths were being rewritten based on the file location which resulted in another relative path. In version 3, the URL is used for rewriting the relative paths which results in another URL.

The diff file is for https://github.com/Polymer/tools.git which seems to fix this issue.

pb3_test.tar.gz
824 bytes Download
pb4_test.tar.gz
844 bytes Download
polymer_bundler.diff
679 bytes Download

Comment 7 by aee@chromium.org, Jan 18 (4 days ago)

polymer-bundler creates a shared_bundle_1.js for the javascript module files. I'm not sure if it can create multiple to maintain module isolation or has some other way to maintain isolation. This newly generated shared_bundle_1.js file needs to be either put into the js file created by crisper or at the very least copied to the output directory with the vulcanized.html and crisper.js files.

crisper does not handle <script> tags with type="module".

https://github.com/PolymerLabs/crisper/blob/master/index.js#L18

We can add type="module" to the query, though we'll also need to add type="module" to the newly created <script> tag.

https://github.com/PolymerLabs/crisper/blob/master/index.js#L68

crisper_test.tar.gz
790 bytes Download

Comment 8 by dpa...@chromium.org, Jan 18 (4 days ago)

> I'm not sure if it can create multiple to maintain module isolation or has some other way to maintain isolation

rollup (which polymer-bundler uses under the cover) can merge multiple modules in a single file while maintaining module isolation. I expect polymer-bundler to be able to do the same, but this is probably a question for Brendan.

I think that after polymer-bundler runs, there should be no <script type="module> in the code, so the fact that crisper does not handle those might be irrelevant.

Also question: Besides Javascript modules, does polymer-bundler 4.0.5 (+the fix for the rewrite-template-in-urls flag) handle our current code (the one without modules)? Can we get this on a CL so that we can try all trybots?


Comment 9 by aee@chromium.org, Jan 19 (4 days ago)

polymer-bundler seems to be doing the right thing. It's respecting the --inline-scripts flag. Maybe crisper needs to use something like rollup in order to handle <script type="module">.

pb4_rollup_test.tar.gz
999 bytes Download

Sign in to add a comment