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

Issue 915053 link

Starred by 2 users

Issue metadata

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

Blocked on:
issue 921285
issue 919916

Blocking:
issue 766694
issue 920264



Sign in to add a comment

WebUI: Add support for Javascript modules

Project Member Reported by dpa...@chromium.org, Dec 14

Issue description

In preparation of moving away from HTML imports, we need to make it possible for WebUI to use Javascirpt modules.  I experimented locally and made some progress:

1) https://chromium-review.googlesource.com/c/chromium/src/+/1375453

This CL fixes the HTTP Content-Type header for chrome:// requests, so that module_fetcher.cc at [1] does not throw an error. It also updates chrome://local-state to use modules as a proof of concept.

2) https://chromium-review.googlesource.com/c/chromium/src/+/1376794

This CL explores the possibility of eliminating dummy HTML files for javascript code that is not holding a Web Component. While it works with optimize_webui=false, it breaks with optimize_webui=true, because polymer-bundler does not handle the modules correctly (needs separate investigation).

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/loader/modulescript/module_script_fetcher.cc?l=57,58
 
Summary: WebUI: Add support for Javascript modules (was: WebUI: Add support Javascript modules)
Blocking: 766694
btw, dpapad@ and I spoke about a potential new syntax that would allow us not to fork all the existing .js files into .m.js versions.  I definitely don't like adding new preprocessor syntaxes, but I dislike the option of forking a lot of code more.

essentially, I took inspiration from things like CONTENT_EXPORT in C++ (content/common/content_export.h) and create little placeholders that have no effect in some cases and useful effects in others.

concretely: hide import/export statements behind comments, e.g. delicious.js:

  /* import "donuts.m.js"; */
  /* import {food} from "microkitchen.m.js"; */

  /* export */ function eatSnacks() {...}

which would generate to delicious.m.js as:

  import "donuts.m.js";
  import {food} from "microkitchen.m.js";

  export function eatSnacks() {...}

and folks would be able to do:

  <script src="donuts.js"></script><!-- or -->
  <script type="module" src="donuts.m.js"></script>

and both would work / stay up-to-date.
> polymer-bundler does not handle the modules correctly (needs separate investigation).

Filed https://github.com/Polymer/tools/issues/814 to track the polymer-bundler related issue.
FYI, some super early experimentation on automatically generating the module files from the existing files at https://chromium-review.googlesource.com/c/chromium/src/+/1385448.

The CL currently handles the following cases
 - Files that don't use cr.define, and just define things on the global scope.
 - Files that do use cr.define and only export a subset of their contents.
Cc: hcarmona@chromium.org
Blockedon: 919916
Blockedon: 921285
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 15

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

commit f9becc9393f93b72dedf43e954a53624371345b2
Author: dpapad <dpapad@chromium.org>
Date: Tue Jan 15 03:39:29 2019

WebUI JS modules: Populate HTTP Content-Type header for chrome:// requests.

This is necessary, otherwise an error is thrown from
third_party/blink/renderer/core/loader/modulescript/module_script_fetcher.cc
when trying to load a JS module from a chrome:// URL.

Bug: 915053
Change-Id: I608dd03592a60b07755b0e1313abf756bd485a8e
Reviewed-on: https://chromium-review.googlesource.com/c/1405496
Reviewed-by: Dan Beam <dbeam@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622714}
[modify] https://crrev.com/f9becc9393f93b72dedf43e954a53624371345b2/content/browser/webui/shared_resources_data_source.cc
[modify] https://crrev.com/f9becc9393f93b72dedf43e954a53624371345b2/content/browser/webui/shared_resources_data_source.h
[modify] https://crrev.com/f9becc9393f93b72dedf43e954a53624371345b2/content/browser/webui/web_ui_data_source_impl.cc
[modify] https://crrev.com/f9becc9393f93b72dedf43e954a53624371345b2/content/browser/webui/web_ui_data_source_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 16

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

commit 51ae704ff187fffb9244f9b6cd208c3011bf3a8a
Author: dpapad <dpapad@chromium.org>
Date: Wed Jan 16 03:14:21 2019

WebUI: Add JS type checking for chrome://local-state

local-state is used as a case study for introducing JS modules to WebUI
because it is a trivially small UI.

In a follow up CL, it will be attempted to use and type-check auto-generated
JS modules.

Bug: 915053
Change-Id: I499e9eb6d5d48e0e377d0f016a6e353436ff1e64
Reviewed-on: https://chromium-review.googlesource.com/c/1413971
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623077}
[modify] https://crrev.com/51ae704ff187fffb9244f9b6cd208c3011bf3a8a/chrome/browser/resources/BUILD.gn
[add] https://crrev.com/51ae704ff187fffb9244f9b6cd208c3011bf3a8a/chrome/browser/resources/local_state/BUILD.gn

Comment 11 by hirosh...@chromium.org, Jan 18 (5 days ago)

Blocking: 920264

Comment 12 by dpa...@chromium.org, Jan 18 (5 days ago)

@hiroshige: I am not sure exactly how much this work will unblock your use-case as I am not very familiar with what you are blocked on exactly. FWIW, the compiler.jar binary in third_party/ already supports modules, and I am also rolling a latest version with better support at  https://crbug.com/922671 .

You can see a proof of concept for WebUI at https://chromium-review.googlesource.com/c/chromium/src/+/1375453, to see whether any of this would be useful for your case?

Sign in to add a comment