New issue
Advanced search Search tips

Issue 882750 link

Starred by 1 user
Status: Started
Owner:
tapted@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ui/file_manager should use strictCheckTypes closure compiler arg

Project Member Reported by tapted@chromium.org, Sep 11 
Chrome Version       : 70.0.3534.4

See also Issue 882739 (for ui/webui/resources/js)

strictCheckTypes appeared in the closure compiler around February 2018 and it seems pretty useful.

Without it, undocumented properties are not type-checked. This means you can remove a documented property from an API when refactoring and not trigger errors in client code that is still using it. Refactoring is hard without strictCheckTypes. With it, it's likely that bugs like  Issue 881680  could be avoided.

It also causes Closure to complain properly when upcasting to {Object}, which effectively disables typechecking.
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 19 

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

commit a0f36c578070e13bae2ff3e5722b3740df3fd709
Author: Trent Apted <tapted@chromium.org>
Date: Wed Sep 19 01:38:41 2018

CrOS Files app: Typecheck the image_loader module with strictCheckTypes.

Bug: 879958, 882750
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ia5992149ad55370a912ed02271dbaa1a93332ff1
Reviewed-on: https://chromium-review.googlesource.com/1226483
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592298}
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/third_party/closure_compiler/README.chromium
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/third_party/closure_compiler/compile_js.gni
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/third_party/closure_compiler/js_unit_tests.gni
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/file_manager/common/js/file_type.js
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/file_manager/common/js/metrics_base.js
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/image_loader/BUILD.gn
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/image_loader/piex_loader.js
[modify] https://crrev.com/a0f36c578070e13bae2ff3e5722b3740df3fd709/ui/file_manager/image_loader/request.js
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 

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

commit 689a69cdc4828221f0c268a49b4eab06e949d92f
Author: Trent Apted <tapted@chromium.org>
Date: Fri Sep 28 01:21:01 2018

Use strictCheckTypes for image_loader unittests.

 - capture unittest_util.js's use of commandLinePrivate
 - audit remaining things using command_line_private.js unnecessarily
   (Only files in file_manager/common/js and file_manager/foreground/js
    use it directly.)
 - Remove an annoying dep that piex_loader_unittest.js has on webui
   (Use a custom element instead: cr/ui.js dislikes strictCheckTypes.).

Bug: 882750, 879958
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I70c9e16b1ba7b9af64e0ce01b7177ef02500c948
Reviewed-on: https://chromium-review.googlesource.com/1245079
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594945}
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/file_manager/common/js/unittest_util.js
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/file_manager/foreground/js/metadata/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/image_loader/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/image_loader/piex_loader_unittest.js
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/689a69cdc4828221f0c268a49b4eab06e949d92f/ui/file_manager/video_player/js/cast/BUILD.gn
►

Sign in to add a comment