closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Undefined $jscomp during Class Inheritence (ES6 to ES5 Strict)

Open evenicoulddoit opened this issue 8 years ago • 45 comments

The following class declaration:

class Job extends JobModelMixin { ... }

Yields the following line when compiled into ES5-Strict,

$jscomp.inherits(Job, JobModelMixin);

But is missing the definition of $jscomp and so throws an error.

evenicoulddoit avatar Sep 14 '15 10:09 evenicoulddoit

Reproducible with the following:

class Foo {}
class Bar extends Foo {
  zulu() {}
}

(beautified)

'use strict';
var Foo = function() {};
var Bar = function(var_args) {
    Foo.apply(this, arguments)
};
$jscomp.inherits(Bar, Foo);
Bar.prototype.zulu = function() {};
//# sourceMappingURL=tmp.min.js.map

evenicoulddoit avatar Sep 14 '15 11:09 evenicoulddoit

Closure Compiler (http://github.com/google/closure-compiler) Version: v20150729 Built on: 2015/08/03 13:40

evenicoulddoit avatar Sep 14 '15 11:09 evenicoulddoit

Can you post the list of command line flags you're using? Maybe there is an option causing the "inject runtime library" step to get skipped.

MatrixFrog avatar Sep 14 '15 16:09 MatrixFrog

Sure - sorry I figured that would help but calling via gulp-closure-compiler so its a little abstracted.

Hopefully this is enough to go off:

{
  compilation_level: 'WHITESPACE_ONLY',
  create_source_map: path.join(ANGULAR.dist, fileName + '.map'),
  source_map_location_mapping: 'angular/|../',
  language_in: 'ECMASCRIPT6_STRICT',
  language_out: 'ECMASCRIPT5_STRICT',
  jscomp_off: ['misplacedTypeAnnotation'],
  output_wrapper: (
    '%output%\n//# sourceMappingURL=' + fileName + '.map'
  )
}

evenicoulddoit avatar Sep 15 '15 07:09 evenicoulddoit

Just switched to SIMPLE_OPTIMIZATIONS and $jscomp is declared.

evenicoulddoit avatar Sep 15 '15 07:09 evenicoulddoit

Currently, WHITESPACE_ONLY will still perform the transpilation, but it won't inject the ES6 runtime library in which $jscomp is defined.

Dominator008 avatar Sep 15 '15 15:09 Dominator008

I think that's intentional so that you can do per-file transpilation. It's just a little weird because if the user asked for transpilation (because the language_in and language_out are different) then obviously we have to do more than just remove whitespace. But it's not obvious that whitespace_only mode skips the runtime library. The "fix" here might just be clearer documentation.

MatrixFrog avatar Sep 15 '15 15:09 MatrixFrog

From my perspective WHITESPACE_ONLY means that the transpiled code is just minified, but not with obfuscation. It would be fine if WHITEPSPACE_ONLY couldn't handle transpilation, but as it can, to not add the required poly-fills would be unexpected?

evenicoulddoit avatar Sep 15 '15 15:09 evenicoulddoit

Maybe perFileTranspilation should be a separate option so that people can explicitly not inject the runtime library when doing so? But then we might need an additional flag for that, ughhh....

I do think it's broken that a WHITESPACE_ONLY whole-program compilation on ES6 code definitely won't work because the runtime library won't be injected.

Dominator008 avatar Sep 15 '15 16:09 Dominator008

I've run into the same problem with library polyfills, and I think we should consider these two cases together, as the es6_runtime is essentially a library polyfilling some language features.

Ideally, I think the output of a transpilation should include some sort of indication (possibly among its goog.requires) that it needs the runtime or certain polyfills to be injected. Ideally this would be handled correctly both by re-compilation as well as by uncompiled/debug loading.

On Tue, Sep 15, 2015 at 9:10 AM Dominator008 [email protected] wrote:

Maybe perFileTranspilation should be a separate option so that people can explicitly not inject the runtime library when doing so?

I do think it's broken that a WHITESPACE_ONLY whole-program compilation on ES6 code definitely won't work because the runtime library won't be injected.

— Reply to this email directly or view it on GitHub https://github.com/google/closure-compiler/issues/1138#issuecomment-140446508 .

shicks avatar Sep 15 '15 19:09 shicks

Just wondering if a resolution is planned for this? The reason that I'm using WHITESPACE_ONLY, is so that during development I can more easily debug (Chrome doesn't actually translate the variables as specified in the sourcemaps which is super disappointing). Essentially that means I can either switch to SIMPLE_OPTIMIZATIONS for development and lose clarity in debugging, or not use inheritance, which is upsetting :(

evenicoulddoit avatar Oct 05 '15 12:10 evenicoulddoit

You know I hate adding more flags but I think this is probably a case where a separate flag to specify explicitly whether to include the runtime library is a good idea.

MatrixFrog avatar Oct 05 '15 16:10 MatrixFrog

Can we chat about it this afternoon? I have a number of thoughts about polyfills in general, and would like to make sure we're on the same page.

shicks avatar Oct 05 '15 16:10 shicks

Was a resolution ever reached on this?

evenicoulddoit avatar Nov 12 '15 10:11 evenicoulddoit

I'd like to see this resolved too.

rhendric avatar Feb 15 '16 07:02 rhendric

@shicks added some new options a while back to have more control over the injection of the runtime library. I'm not sure if they're all exposed in the open source command line runner yet.

MatrixFrog avatar Feb 17 '16 16:02 MatrixFrog

@MatrixFrog hey, just wondering if this has been added yet? Trying to transpile es6 code in WHITESPACE_ONLY mode and $jscomp is undefined.

Alternatively if there's a way to manually inject this somehow that would also be fine for the time being.

jdb8 avatar Mar 12 '16 00:03 jdb8

@jd8 var $jscomp is defined at https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/js/base.js#L26. $jscomp.inherits is defined at https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/js/es6_runtime.js#L91. You can manually inject those files for now.

Dominator008 avatar Mar 12 '16 00:03 Dominator008

@Dominator008 thanks for the quick response! Would be great if a flag could be added to do this for us (include the runtime during WHITESPACE_ONLY compilation) to simplify things.

jdb8 avatar Mar 12 '16 00:03 jdb8

@jdb8 There's a flag --noinject_library to prevent the injection from happening, and a compiler option forceLibraryInjection to force the injection. Unfortunately that option is not available via the command line (yet).

@shicks @MatrixFrog Should we just remove && !options.skipNonTranspilationPasses from https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/DefaultPassConfig.java#L314? I suspect it requires some internal cleanup but that should be the way to go? An alternative would be exposing the forceLibraryInjection to the command line, which I guess is done internally already?

Dominator008 avatar Mar 12 '16 00:03 Dominator008

@Dominator008 great, we managed to get it working after adding the --noinject_library=es6_runtime for ADVANCED mode (since otherwise the runtime was being injected twice which caused issues). We also had to switch to using the LOOSE dependency mode to avoid the es6_runtime.js and base.js files from being dropped, due to their lack of goog.provides.

Some commandline flag that could be used to achieve all this would be great to simplify the logic in our wrapper code. Otherwise perhaps some documentation on these steps, as well as a note on the es6 page that the runtime is needed for WHITESPACE_ONLY mode, would be very helpful.

jdb8 avatar Mar 12 '16 23:03 jdb8

There are a few flags that are only being used by a single project internally. Sometime fairly soon we'll establish the "right way" for projects to do file-by-file transpilation and then we can do a lot of flag cleanup. Unfortunately there are a few other things we'll probably need to focus on first.

MatrixFrog avatar Mar 14 '16 16:03 MatrixFrog

@MatrixFrog no worries, thanks for looking into this! Just wanted to add the solution we ended up going with in case anyone else runs into the same issue before things get updated.

jdb8 avatar Mar 15 '16 00:03 jdb8

+1 on getting a new command line option for --noinject_library while still remapping the calls to polyfillable features.

caridy avatar Mar 29 '16 07:03 caridy

Alternatively, the closure-library now seems to ship with transpile.js , which can be manually injected in the meantime. It provides the $jscomp and appears to work.

lewistg avatar Aug 04 '16 03:08 lewistg

Polyfills are no longer remapped, and you probably don't want to inject the entire 600k obfuscated transpiler just to get a few kilobytes of polyfills.

I believe --noinject_libraries has been working for a while (though its documentation might be a little out of whack - the option listed in CommandLineRunner.categories is noinject_library, which is wrong).

I had thought --inject_library was also available but I'm not seeing it right now. Maybe I'm missing something. In any case, if it's missing, it's certainly worth adding.

shicks avatar Aug 04 '16 03:08 shicks

To clarify, in the past we rewrote calls to new Map as new $jscomp.Map - we don't do that anymore. Now all that RewritePolyfills pass does is inject a runtime library to ensure that the global Map symbol is defined and correct.

shicks avatar Aug 04 '16 04:08 shicks

I was just hit by this I think. I have a mixed project where I have some goog.module's with ES6 classes and some old-school goog.provide files with goog.inherits.

With ES6 to ES5, full compilation, I see separate $jscomp$inherits and goog$inherits functions. They seem similar, both are defined and they work. I'm curious, do they actually do different things? Could you just use one of them for both cases?

When I change compilation to whitespace only, goog.inherits is defined and used, but $jscomp$inherits is used without definition.

Is there a workaround currently available for this? I see several options above but not sure which would be the most future proof, I'm too new to decode this long conversation.

fejesjoco avatar Oct 07 '16 16:10 fejesjoco

Are you using the internal or the public compiler? If internal, I don't know what "whitespace only" means, since that's not a flag as I understand it. It hasn't always been the case, but my understanding is that for the last few months at least, whitespace only has included runtime injection, but I could be misremembering.

To address the question of the difference, goog.inherits is for Closure Library, and is related to the assumptions Closure Library makes about class structure (I don't recall what these are first-hand). $jscomp.inherits is an ES6-to-ES5 polyfill for the ES6 'extends' keyword. One important difference is that ES6's extends keyword causes both instance properties and static properties to be inherited, so $jscomp.inherits actually copies the static properties from the superclass to the subclass (this isn't strictly correct, but there's not really a reliable way to make a constructor with a prototype other than Function using non-ES6 features), whereas goog.inherits does not do that. That said, the style guide frowns on so-called static inheritance, so it should generally not be relied on.

On Fri, Oct 7, 2016 at 9:53 AM, fejesjoco [email protected] wrote:

I was just hit by this I think. I have a mixed project where I have some goog.module's with ES6 classes and some old-school goog.provide files with goog.inherits.

With ES6 to ES5, full compilation, I see separate $jscomp$inherits and goog$inherits functions. They seem similar, both are defined and they work. I'm curious, do they actually do different things? Could you just use one of them for both cases?

When I change compilation to whitespace only, goog.inherits is defined and used, but $jscomp$inherits is used without definition.

Is there a workaround currently available for this? I see several options above but not sure which would be the most future proof, I'm too new to decode this long conversation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/closure-compiler/issues/1138#issuecomment-252304234, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIyG53SOVdMESlRG2_qM1Gd9o2wrQRoks5qxnkKgaJpZM4F8wKg .

shicks avatar Oct 19 '16 07:10 shicks

whitespace-only mode is used for file-by-file transpilation, and in those cases we don't want the runtime libraries injected in every file. There's not currently a flag to tell it to inject libraries as needed i whitespace-only mode, but you can explicitly request the "es6_runtime" library be injected, which should take care of your case.

shicks avatar Oct 28 '16 21:10 shicks