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

ES6 Builds Broken by #6a5aec8 #15979

Closed
godenji opened this issue Oct 17, 2018 · 12 comments · Fixed by #16207 or #16417
Closed

ES6 Builds Broken by #6a5aec8 #15979

godenji opened this issue Oct 17, 2018 · 12 comments · Fixed by #16207 or #16417
Assignees

Comments

@godenji
Copy link

godenji commented Oct 17, 2018

Bug Report

Ionic Info

Ionic:

   ionic (Ionic CLI)             : 4.2.1
   Ionic Framework               : @ionic/angular 4.0.0-beta.13
   @angular-devkit/build-angular : 0.7.5
   @angular-devkit/schematics    : 0.7.5
   @angular/cli                  : 6.2.5
   @ionic/angular-toolkit        : 1.0.0

Cordova:

   cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
   Cordova Platforms     : android 7.1.1
   Cordova Plugins       : cordova-plugin-ionic-keyboard 2.1.2, cordova-plugin-ionic-webview 2.0.3, (and 8 other plugins)

System:

   NodeJS : v10.10.0 (/usr/bin/node)
   npm    : 6.4.1
   OS     : Linux 4.16

Describe the Bug
Commit #6a5aec8 breaks ES6 targeted builds.

Steps to Reproduce
Steps to reproduce the behavior:

  1. Upgrade to v4 beta 13
  2. Set tsconfig.json compile target to es6
  3. run ionic serve

Expected Behavior
ES6 targeted builds are produced successfuly.

Additional Context

TypeError: "class constructors must be invoked with |new|"
	BackButtonEmitter http://localhost:8100/vendor.js:129680:35
	Platform http://localhost:8100/vendor.js:129695:27
	_createClass http://localhost:8100/vendor.js:63003:20
	_createProviderInstance http://localhost:8100/vendor.js:62970:26
	resolveNgModuleDep http://localhost:8100/vendor.js:62915:21
	get http://localhost:8100/vendor.js:64022:16
	inject http://localhost:8100/vendor.js:54331:16
	injectArgs http://localhost:8100/vendor.js:54371:23
	convertInjectableProviderToFactory http://localhost:8100/vendor.js:54440:34
	_callFactory http://localhost:8100/vendor.js:63030:20
	_createProviderInstance http://localhost:8100/vendor.js:62973:26
	resolveNgModuleDep http://localhost:8100/vendor.js:62932:17
	_callFactory http://localhost:8100/vendor.js:63036:106
	_createProviderInstance http://localhost:8100/vendor.js:62973:26
	initNgModule http://localhost:8100/vendor.js:62876:32
	NgModuleRef_ http://localhost:8100/vendor.js:64005:9
	createNgModuleRef http://localhost:8100/vendor.js:63988:12
	debugCreateNgModuleRef http://localhost:8100/vendor.js:66419:12
	create http://localhost:8100/vendor.js:67513:16
	bootstrapModuleFactory http://localhost:8100/vendor.js:57837:49
	invoke http://localhost:8100/polyfills.js:2710:17
	onInvoke http://localhost:8100/vendor.js:57262:24
	invoke http://localhost:8100/polyfills.js:2709:17
	run http://localhost:8100/polyfills.js:2460:24
	run http://localhost:8100/vendor.js:57149:54
	bootstrapModuleFactory http://localhost:8100/vendor.js:57833:16
	bootstrapModule http://localhost:8100/vendor.js:57884:38
	invoke http://localhost:8100/polyfills.js:2710:17
	run http://localhost:8100/polyfills.js:2460:24
	scheduleResolveOrReject http://localhost:8100/polyfills.js:3194:29
	invokeTask http://localhost:8100/polyfills.js:2743:17
	runTask http://localhost:8100/polyfills.js:2510:28
	drainMicroTaskQueue http://localhost:8100/polyfills.js:2917:25

Generated JS looks like:

var BackButtonEmitter = /** @class */ (function (_super) {
    __extends(BackButtonEmitter, _super);
    function BackButtonEmitter() {
        return _super !== null && _super.apply(this, arguments) || this;
    }
    BackButtonEmitter.prototype.subscribeWithPriority = function (priority, callback) {
        return this.subscribe(function (ev) {
            ev.register(priority, callback);
        });
    };
    return BackButtonEmitter;
}(_angular_core__WEBPACK_IMPORTED_MODULE_0__["EventEmitter"]));

and build blows up with "class constructors must be invoked with |new|" on this call: _super.apply(this, arguments).

@mhartington
Copy link
Contributor

For reference, this is the commit in question.

6a5aec8

@manucorporat any thoughts on this?

@hal1984
Copy link
Contributor

hal1984 commented Oct 31, 2018

Same problem

@sanja-h
Copy link

sanja-h commented Nov 1, 2018

Still the same problem with @ionic/angular@4.0.0-beta.15-0.

@godenji
Copy link
Author

godenji commented Nov 1, 2018

Still the same problem with @ionic/angular@4.0.0-beta.15-0

That's because the author of the commit that introduced the regression is clearly working on other, presumably more important, issues :)

A quick and dirty hack that "fixes" the issue (i.e. reverts to old behavior) is to change line 32 of node_modules/@ionic/angular/dist/providers/platform.js from
this.backButton = new BackButtonEmitter(); to this.backButton = new EventEmitter();

manucorporat added a commit that referenced this issue Nov 3, 2018
manucorporat added a commit that referenced this issue Nov 3, 2018
@ChrisPearce
Copy link

Thank you for the fix.

Is the v4.0.0-beta.16 release scheduled to happen soon? It would be very helpful for users whose codebases are ES6 based to use this as soon as possible.

@paulstelzer
Copy link
Contributor

@ChrisPearce Just fork this repository and build it - so you always get the newest version and do not have to wait until the next beta ;)

@paulstelzer
Copy link
Contributor

paulstelzer commented Nov 17, 2018

@manucorporat This issue is still available in beta.16. Your fix is not working. The error is now: "TypeError: Class constructor EventEmitter cannot be invoked without 'new'"

@ChrisPearce
Copy link

ChrisPearce commented Nov 18, 2018

Is ES6/ES2015 a supported scenario for Ionic v4, or should we only be targeting ES5?

Also, is it intended to support ES2016 and ES2017 (when a mobile device supports these)?

@godenji
Copy link
Author

godenji commented Nov 18, 2018

Is ES6/ES2015 a supported scenario for Ionic v4, or should we only be targeting ES5?

I don't see why not, the issue is isolated to a specific area of the code base -- fix is likely trivial.

@manucorporat
Copy link
Contributor

manucorporat commented Nov 19, 2018

damn.

We are working in a new test app for angular and a CI that runs it in all different browsers to make sure we don't have this kind of issues anymore, thanks you all for the patience!

Reopening issue

@paulstelzer
Copy link
Contributor

Can confirm. es6 build is now possible :)

@ionitron-bot
Copy link

ionitron-bot bot commented Dec 22, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Dec 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants