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

ngx-tour-ng-bootstrap broken with es6 #77

Closed
dhalperi opened this issue Jan 25, 2018 · 3 comments
Closed

ngx-tour-ng-bootstrap broken with es6 #77

dhalperi opened this issue Jan 25, 2018 · 3 comments

Comments

@dhalperi
Copy link

Hey,

I'm finding that ngx-tour does not work for Angular projects set to target es6. In these cases, Angular apparently fails to inject the Router when constructing the NgbTourService, leading to this trace:

ng:///AppModule/AppComponent_Host.ngfactory.js:5 ERROR TypeError: Cannot read property 'events' of undefined
    at NgbTourService.TourService.startAt (webpack-internal:///../../../../ngx-tour-ng-bootstrap/ngx-tour-ng-bootstrap.js:97)
    at NgbTourService.TourService.start (webpack-internal:///../../../../ngx-tour-ng-bootstrap/ngx-tour-ng-bootstrap.js:86)
    at AppComponent.ngOnInit (webpack-internal:///../../../../../src/app/app.component.ts:34)
    at checkAndUpdateDirectiveInline (webpack-internal:///../../../core/esm2015/core.js:10507)
    at checkAndUpdateNodeInline (webpack-internal:///../../../core/esm2015/core.js:12030)
    at checkAndUpdateNode (webpack-internal:///../../../core/esm2015/core.js:11973)
    at debugCheckAndUpdateNode (webpack-internal:///../../../core/esm2015/core.js:12852)
    at debugCheckDirectivesFn (webpack-internal:///../../../core/esm2015/core.js:12797)
    at Object.eval [as updateDirectives] (ng:///AppModule/AppComponent_Host.ngfactory.js:9)
    at Object.debugUpdateDirectives [as updateDirectives] (webpack-internal:///../../../core/esm2015/core.js:12786)

Deeper reproduction instructions below.

  1. Is this a known issue?
  2. Is this expected, or a bug?

es6 is pretty nice, and Angular has supported it since September -- ng-cli version v1.5.0.

Thanks!


This repo https://github.com/dhalperi/ngx-tour-bug has three commits:

  1. Create a stock Angular project ng-cli new testtour.
  2. Add the simplest possible version of ngx-tour-ng-bootstrap
    (https://github.com/dhalperi/ngx-tour-bug/commit/5e3451ce02e85c162392cab17e42e27db0b18e91).
  3. Swap the target from es5 to es6 (https://github.com/dhalperi/ngx-tour-bug/commit/2f44c1d6a5e2ebee58476a0136495434e5245cca).

After the second commit, ngx-tour-ng-bootstrap seems to work great.

After switching to es6, the library fails as described above -- see the developer console error messages.

@isaacplmann
Copy link
Owner

isaacplmann commented Jan 25, 2018

Thanks for the thorough bug report. It's very nice to have a simple repo that reproduces the problem.

I submitted a PR to your repo that switch to using the ngx-tour-md-menu version - which apparently works in es6 mode. I really don't know why the ng-bootstrap version would not work in es6 mode. At first I thought it might be the underlying ng-bootstrap library itself, but the error is happening in the ngx-tour code.

And as I was typing up the response I had another idea.

This workaround will work. I'm not sure why it's necessary only for es6 though.

import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { TourNgBootstrapModule, TourService as NgbTourService } from 'ngx-tour-ng-bootstrap';
import { TourModule, TourService } from 'ngx-tour-core';
import { RouterModule } from '@angular/router';

import { AppComponent } from './app.component';

const routing = RouterModule.forRoot([]);

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    routing,
    TourNgBootstrapModule.forRoot(),
    TourModule.forRoot(),
  ],
  providers: [
    { provide: NgbTourService, useExisting: TourService },
  ],
  bootstrap: [AppComponent]
})
export class AppModule { }

@dhalperi
Copy link
Author

Hey guess what, the bug is in Angular.

Specifically, Angular only supports ES6 properly in AOT mode which is used neither by default (ng serve defaults to JIT, to enable eot add --aot) nor is even possible to use for tests.

I think this means we have to target ES5 until Angular either stops using JIT for tests or supports ES6 in JIT mode.

My gut feeling now is that we've been getting lucky so far that our tests haven't been broken by our use of ES6. The repro here happens to be one case where one of the known ES6+JIT bugs crops up.

So we do have to go make our code target ES5 if we want our tests to reliably pass, even though we can use ES6 in our prod builds (which use AOT).

@dhalperi
Copy link
Author

Thanks @isaacplmann for digging and for the workaround -- I think that since we've just been getting lucky so far, we'll port back to es5 where I am optimistic the bug won't show up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants