Skip to content

Router outlet broken #166

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

Closed
filipesilva opened this issue Feb 1, 2016 · 20 comments · Fixed by #171
Closed

Router outlet broken #166

filipesilva opened this issue Feb 1, 2016 · 20 comments · Fixed by #171

Comments

@filipesilva
Copy link
Contributor

The last two commits (e2c8795 and 2efc9a1) seem to have broken the router. Somehow, ngOnInit events on child routes are not firing.

Undoing these commits causes them to fire again. I suspect this might be the issue, in that dependencies are loaded out of order ( previous order ) and thus cause parts of angular to not have the required dependencies.

/cc @cironunes @jkuri

@jkuri
Copy link
Contributor

jkuri commented Feb 1, 2016

Hi, do you maybe have any example ready to test with?

@jkuri
Copy link
Contributor

jkuri commented Feb 1, 2016

Maybe just Rx should be loaded before Angular2, can you check please?

@filipesilva
Copy link
Contributor Author

I ran into this with routes generated via #139. The ngOnInit on hero-detail.component.ts and hero-list.component.ts (assuming you use ng generate route hero) do not work with those two commits.

I am unsure of how to just load Rx before the rest with broccoli though, how do I do it?

@jkuri
Copy link
Contributor

jkuri commented Feb 1, 2016

Just add this line before **/angular2.dev.js. Then try to generate routes again.

@filipesilva
Copy link
Contributor Author

That did not work.

I used the order on the quickstart and it works:

      inputFiles: [
        // '**/es6-shim.js',
        '**/system-polyfills.js',
        '**/angular2-polyfills.js',
        '**/system.src.js',
        '**/Rx.js',
        '**/angular2.dev.js',
        '**/http.dev.js',
        '**/router.dev.js',
        '**/upgrade.dev.js'
      ],

...except that the es6-shim errors. If I take it out it's fine.

I can't see why it would error though, since its whole purpose is the be the very first think so it can shim missing functionality.

@jkuri
Copy link
Contributor

jkuri commented Feb 1, 2016

Okay, then just add this fix to your PR. I will check about es6-shim errors
asap.

On Monday, 1 February 2016, Filipe Silva [email protected] wrote:

That did not work.

I used the order on the quickstart
https://angular.io/docs/ts/latest/quickstart.html#!%23add-the-index-html-
and it works:

  inputFiles: [
    // '**/es6-shim.js',
    '**/system-polyfills.js',
    '**/angular2-polyfills.js',
    '**/system.src.js',
    '**/Rx.js',
    '**/angular2.dev.js',
    '**/http.dev.js',
    '**/router.dev.js',
    '**/upgrade.dev.js'
  ],

...except that the es6-shim errors. If I take it out it's fine.

I can't see why it would error though, since its whole purpose is the be
the very first think so it can shim missing functionality.


Reply to this email directly or view it on GitHub
#166 (comment)
.

@filipesilva
Copy link
Contributor Author

Tried adding the shim separately, outside vendor.js and it works fine.

I'd prefer not to change up that order from within my PR because it's not related to routes per se, but rather to dependency loading.

I'll gladly open up a new PR for it though, but might as well also fix the shim thing.

@jkuri
Copy link
Contributor

jkuri commented Feb 2, 2016

Hi @filipesilva. Is this working now?

@filipesilva
Copy link
Contributor Author

No, still has the problem with the shim being loaded via broccolli. I haven't had time to look into it more.

@jkuri
Copy link
Contributor

jkuri commented Feb 2, 2016

Please try the following order, if it works for you;

inputFiles: [
  '**/angular2-polyfills.js',
  '**/system.src.js',
  '**/system-polyfills.js',
  '**/es6-shim.js',
  '**/Rx.js',
  '**/angular2.dev.js',
  '**/http.dev.js',
  '**/router.dev.js',
  '**/upgrade.dev.js',
],

if its okay please provide PR with the fix, otherwise please tell me how to reproduce an error.

@cironunes
Copy link
Member

any news?

@filipesilva
Copy link
Contributor Author

@jkuri just tried the order you said, still does not work.

We have to follow the order in the guides though: es6-shim.js needs to be the first one, system-polyfills.js before system, etc. Otherwise those files aren't doing anything.

To reproduce, do the following:

ng new testproj
cd testproj
npm link angular-cli
ng g route hero
// follow the instructions on readme, line 95 to add the route
ng serve
// open browser on http://localhost:4200/hero

This is what appears:

image

Now open angular-cli/lib/broccoli/angular2-app.js and change lines 59-69 to have the order indicated in the quickstart:

      inputFiles: [ //TODO: figure out how to make it a glob that maintains the order of the files
        '**/es6-shim.js',
        '**/system-polyfills.js',
        '**/angular2-polyfills.js',
        '**/system.src.js',
        '**/Rx.js',
        '**/angular2.dev.js',
        '**/http.dev.js',
        '**/router.dev.js',
        '**/upgrade.dev.js'
      ],

Stop and restart ng serve. Browser should have an error:

image

Comment out the es6-shim.js line, stop and restart ng serve. Browser will show the correct result:

image

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Feb 2, 2016
@jkuri
Copy link
Contributor

jkuri commented Feb 2, 2016

@filipesilva thanks for helping with reproduce, I got same errors.
Solved it with this order;

'**/angular2-polyfills.js',
'**/es6-shim.js',
'**/system-polyfills.js',
'**/system.src.js',
'**/Rx.js',
'**/angular2.dev.js',
'**/http.dev.js',
'**/router.dev.js',
'**/upgrade.dev.js'

I think adding '**/angular2-polyfills.js' before all others is the key (not sure). Please check.

@filipesilva
Copy link
Contributor Author

@jkuri that order doesn't fix it for me... are you sure you didn't do something else? I tried using that order on a fresh copy of master. I've notice that if you restart ng-serve but don't refresh the browser, sometimes it works. I don't understand why.

I made a #170 where es6-shim.js is loaded separately from the rest of the libs, and it seems to fix this issue. But I don't see why loading it separately should change anything.

@jkuri
Copy link
Contributor

jkuri commented Feb 2, 2016

Hm, guess I ran into that case you mention. I don't understand neither. Maybe some broccolli concat issue? Don't know.

@filipesilva
Copy link
Contributor Author

I even thought it might be related to the ember-cli auto reload somehow and changed the order of that with no results. Really weird issue....

@jkuri
Copy link
Contributor

jkuri commented Feb 3, 2016

I gave this issue one more shot, I found order which works properly with ng g route hero command (always). @filipesilva please check, I think this one is better than #170 as it not includes es6-shim.js separately.

@filipesilva
Copy link
Contributor Author

@jkuri I tested your PR and it works. I also agree it's better because of what you said.

There's still something fishy going on with broccolli and importing the es6-shim but I have no clue as to what it is right now.

@cironunes can you have a look at the PR before I merge it?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants
@cironunes @jkuri @filipesilva @erdemildiz and others