Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Closure In Build Tools #1852

Closed
vaage opened this issue Mar 27, 2019 · 1 comment
Closed

Update Closure In Build Tools #1852

vaage opened this issue Mar 27, 2019 · 1 comment
Labels
status: archived Archived and locked; will not be updated type: code health A code health issue

Comments

@vaage
Copy link
Contributor

vaage commented Mar 27, 2019

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.

This is related to the problem described in #1792 but more over, a newer version of closure will contain updated polyfills and stricter type detection/enforcement.

Describe the solution you'd like

If possible, updating to the newest stable version would be ideal. But if not possible, updating to a newer version number would provide more-than-minor benefits in terms of polyfils and type-checking.

Describe alternatives you've considered

We have regularly talked about alternative solutions for type-checking (such as using typescript) but so far they would require too many resources and would not necessarily resolve polyfill issues.

Additional context

@vaage vaage added the type: code health A code health issue label Mar 27, 2019
@vaage vaage added this to the v2.6 milestone Mar 27, 2019
shaka-bot pushed a commit that referenced this issue Jun 3, 2019
Found by a newer compiler: lib/debug/log.js should be permitted to use
console directly.  It's not clear why the older compiler was okay with
this without the exception.  Perhaps the type information was missing
before the upgrade.

Issue #1852

Change-Id: I3370bea924428539c860db370c29878433a27d38
shaka-bot pushed a commit that referenced this issue Jun 3, 2019
Two patches in the IE EME polyfill became undefined during the ES6
conversion.  These mistakes were caught by a newer compiler.

setMediaKeys was declared on a different class (MediaKeySystemAccess).
And both setMediaKeys and requestMediaKeySystemAccess needed to be
declared static in order to be referenced from a static install
method.

Issue #1852

Change-Id: Ibbeb57bb3f25e84826ab094681717fdaf1b5a8b7
shaka-bot pushed a commit that referenced this issue Jun 3, 2019
Style properties like width, height, and paddingTop should be set on
the style object, not the element directly.  Further, "float" is the
wrong name for the css float property in JavaScript.  This should be
"cssFloat".

These bugs were caught by a newer compiler.

Issue #1852

Change-Id: I80e8b98a7b693b2b6bb0c52ff9f4a0611e3ea106
shaka-bot pushed a commit that referenced this issue Jun 3, 2019
These inconsistencies were caught by a newer compiler.

Issue #1852

Change-Id: I2ee470f265bee726eb71ee92f442feb328c457a4
shaka-bot pushed a commit that referenced this issue Jun 3, 2019
These modules are loaded by the tests w/ require() and assigned to
window, but the externs declare the namespaces as "const".  If they
are declared const, then assigning them in test/test/boot.js is a
problem for the newer compiler.  To work around this, assign them
using bracket notation.

Issue #1852

Change-Id: I2319ada7cdf502b5f1a5a3e65e9491c666404c89
joeyparrish added a commit that referenced this issue Jun 4, 2019
Found by a newer compiler: lib/debug/log.js should be permitted to use
console directly.  It's not clear why the older compiler was okay with
this without the exception.  Perhaps the type information was missing
before the upgrade.

Issue #1852

Change-Id: I3370bea924428539c860db370c29878433a27d38
joeyparrish added a commit that referenced this issue Jun 4, 2019
Style properties like width, height, and paddingTop should be set on
the style object, not the element directly.  Further, "float" is the
wrong name for the css float property in JavaScript.  This should be
"cssFloat".

These bugs were caught by a newer compiler.

Issue #1852

Change-Id: I80e8b98a7b693b2b6bb0c52ff9f4a0611e3ea106
joeyparrish added a commit that referenced this issue Jun 4, 2019
These inconsistencies were caught by a newer compiler.

Issue #1852

Backported to v2.5.x

Change-Id: I2ee470f265bee726eb71ee92f442feb328c457a4
joeyparrish added a commit that referenced this issue Jun 4, 2019
These modules are loaded by the tests w/ require() and assigned to
window, but the externs declare the namespaces as "const".  If they
are declared const, then assigning them in test/test/boot.js is a
problem for the newer compiler.  To work around this, assign them
using bracket notation.

Issue #1852

Backported to v2.5.x

Change-Id: I2319ada7cdf502b5f1a5a3e65e9491c666404c89
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
Found by a newer compiler: lib/debug/log.js should be permitted to use
console directly.  It's not clear why the older compiler was okay with
this without the exception.  Perhaps the type information was missing
before the upgrade.

Issue shaka-project#1852

Change-Id: I3370bea924428539c860db370c29878433a27d38
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
Two patches in the IE EME polyfill became undefined during the ES6
conversion.  These mistakes were caught by a newer compiler.

setMediaKeys was declared on a different class (MediaKeySystemAccess).
And both setMediaKeys and requestMediaKeySystemAccess needed to be
declared static in order to be referenced from a static install
method.

Issue shaka-project#1852

Change-Id: Ibbeb57bb3f25e84826ab094681717fdaf1b5a8b7
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
Style properties like width, height, and paddingTop should be set on
the style object, not the element directly.  Further, "float" is the
wrong name for the css float property in JavaScript.  This should be
"cssFloat".

These bugs were caught by a newer compiler.

Issue shaka-project#1852

Change-Id: I80e8b98a7b693b2b6bb0c52ff9f4a0611e3ea106
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
These inconsistencies were caught by a newer compiler.

Issue shaka-project#1852

Change-Id: I2ee470f265bee726eb71ee92f442feb328c457a4
AnteWall pushed a commit to AnteWall/shaka-player that referenced this issue Jul 17, 2019
These modules are loaded by the tests w/ require() and assigned to
window, but the externs declare the namespaces as "const".  If they
are declared const, then assigning them in test/test/boot.js is a
problem for the newer compiler.  To work around this, assign them
using bracket notation.

Issue shaka-project#1852

Change-Id: I2319ada7cdf502b5f1a5a3e65e9491c666404c89
@joeyparrish joeyparrish modified the milestones: v2.6, Backlog Feb 12, 2020
@joeyparrish
Copy link
Member

This has been completed for some time already, but we forgot to tag & close the issue.

We now pull Closure Compiler and the Closure Library from npm, and updating is relatively easy. We are no longer years behind.

@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Nov 28, 2021
@shaka-project shaka-project locked and limited conversation to collaborators Nov 28, 2021
@avelad avelad removed this from the Backlog milestone May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: code health A code health issue
Projects
None yet
Development

No branches or pull requests

4 participants