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

feat(aot):export default routes array to work with AoT compiler #2680

Closed
wants to merge 1 commit into from

Conversation

Meligy
Copy link
Contributor

@Meligy Meligy commented Oct 13, 2016

The AoT compiler needs the variables used in the @NgModule declaration to be exported as well.

This doesn't fail when the array is empty (come compiler optimization magic maybe?), but as soon as the user enters their first route into the array, their first experience with AoT becomes an error. So, this improves the default to be compatible with AoT compiler from the beginning.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@Meligy
Copy link
Contributor Author

Meligy commented Oct 13, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@Brocco
Copy link
Contributor

Brocco commented Oct 13, 2016

Can you please create an e2e test to verify?

@Meligy
Copy link
Contributor Author

Meligy commented Oct 13, 2016

Hmm. Can you help me with planning this one? I'm not sure what the e2e test would do, given:

  • The CLI does not add routes to the route array by itself. Adding routes is disabled at the moment, and I imagine might be unneeded even (generating modules with --routing is good enough for CLI tooling)
  • The error only happens when the user manually adds a route object to the routes array. The empty array does not cause an AoT compiler error

@Meligy
Copy link
Contributor Author

Meligy commented Oct 14, 2016

I had a thought that maybe I can test that routes is being exported, by maybe require()ing the file and checking for the routes property.

But then I looked at the e2e tests (in tests/e2e/tests/generate), and it seemed that all tests only ensure files are created, no checks for the shapes of the files.

Am I missing something, or would this be good to go?

Thanks a lot.

@filipesilva
Copy link
Contributor

Heya @Meligy, could you give me some more context on the issue that the PR aims to resolve? That sounds like a AoT compiler limitation, but one that I was not aware of. Is there an issue on angular/angular maybe?

Regarding tests, our test infrastructure just restores everything after each test, so you should be fine manually creating some files and replacing the import statement. An example of something of this sort can be found in https://github.com/angular/angular-cli/blob/master/tests/e2e/tests/build/styles/scss.ts

@Meligy
Copy link
Contributor Author

Meligy commented Oct 16, 2016

When you don't export the routes, and you add an actual module route (lazy or not), trying to compile with --aot fails with something

Error encountered resolving symbol values statically. Reference to a local (non-exported) symbol 'routes'. Consider exporting the symbol (position 5:7 in the original .ts file), resolving symbol AppRoutingModule in /Users/Meligy/Code/github/Meligy/routing-angular-cli/src/app/app-routing.module.ts

There`re several related issues in the @angular/angular repository. I can't seem to find the most accurate issue unfortunately.

I have a simple repro at @Meligy/routing-angular-cli.

FYI, this only solves build issues (output when you run the command). AoT routing (components, bundling other modules, and lazy loaded modules) is broken in runtime still (browser console), which I'm working on improving my repro to show all cases, and then will see if I can add to existing issue or create new one for.

@Meligy
Copy link
Contributor Author

Meligy commented Oct 20, 2016

The problem this PR fixes seems to be gone in Angular 2.1.0, so, I'm closing this PR.

The problem explained in #2735 though is still there.

The difference if not clear is:

  • The problem this PR was fixing was happening in the terminal console when running ng build --aot or ng serve --aot

    Keyword here is "terminal"

  • The problem Routing not working with AoT #2735 explains is a problem that still happens in the browser when running ng serve --aot

    Keyword here is "browser console"

@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 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants