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(NgModule) consider removing the need to list every routing-component in declarations #10472

Closed
choeller opened this issue Aug 3, 2016 · 18 comments
Labels
area: router needs reproduction This issue needs a reproduction in order for the team to investigate further

Comments

@choeller
Copy link
Contributor

choeller commented Aug 3, 2016

I'm submitting a ... (check one with "x")

[ ] bug report
[x] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Before NgModules I only had to import my routes in the routes config:

import {DashboardComponent} from "./dashboard/dashboard.component";
import {SettingsComponent} from "./settings/settings.component";
import {AboutComponent} from "./about/about.component";
import {LoginComponent,LoginGuard,UserResolver} from "./login/index";
import {NotFoundComponent} from "./not-found/not-found.component";
import {TasksComponent} from './tasks/tasks.component';

export const APP_ROUTES: Routes = [
  {path: '', component: DashboardComponent}, 
  {path: 'settings', component: SettingsComponent}, 
  {path: 'about', component: AboutComponent}
];

after upgrading to NgModule I need to import this whole list once again in my bootstrap-file / MainModule:

import {DashboardComponent} from "./dashboard/dashboard.component";
import {SettingsComponent} from "./settings/settings.component";
import {AboutComponent} from "./about/about.component";
import {LoginComponent,LoginGuard,UserResolver} from "./login/index";
import {NotFoundComponent} from "./not-found/not-found.component";
import {TasksComponent} from './tasks/tasks.component';

@NgModule({
  imports: [BrowserModule, FormsModule, RouterModule.forRoot(APP_ROUTES)],
  entryComponents: [AppComponent],
  declarations: [DashboardComponent, SettingsComponent,  AboutComponent, LoginComponent, NotFoundComponent, TasksComponent ],
  bootstrap: [AppComponent]
})
class MainModule {
}

This is quite a lot of copy paste...

Expected/desired behavior

I would assume, that when I pass the routing-config to the RouterModule, it should be possible for Angular to add the Routing-Components (like DashboardComponent) automatically to the module.

Whereas for other Directives the NgModule-approach means less work, for Routing-Components it currently means more work than before. So it would be cool to reduce the need of copy-pasting here.

Please tell us about your environment:

  • Angular version: 8efbcc9
  • Browser: [all ]
  • Language: [all]
@damiandennis
Copy link

I added an export in my routes with an array of all my components, saves importing again in bootstrap however still a pain to maintain.

@DaSchTour
Copy link

I already wondered which components need to be added to the module. Since every component has it's own dependencies I don't quite get why there is a need of redeclaring everything in this module definition.

@choeller
Copy link
Contributor Author

@DaSchTour

Since every component has it's own dependencies...

This is the biggest breaking change of the new NgModule - Architecture: Starting from RC.6 components will not have there own dependencies, but dependencies will have to be managed on a Module-Level. RC-5 already works like this, but supports using directives and pipes array on the component to allow for an easier transition. under the hood already now all dependencies are shift up to the module.

@DzmitryShylovich
Copy link
Contributor

why there is a need of redeclaring everything in this module definition

lazy loading

@damiandennis
Copy link

@choeller I hope that is not the case. That will be a huge change and tons of refactoring.... I thought RC was meant to be fairly stable.

@DaSchTour
Copy link

I added my thoughts on all this in #10552
This looks like going back to angular 1 and totally breaks DRY
Hides dependencies and will lead to "Better add everything to the module, you might not know if you will need it"
I guess every project will have a const ALL_MODULES = […] and add it just everywhere.
I really fear this might lead to very very ugly solutions.

@choeller
Copy link
Contributor Author

@damiandennis well, currently it is case - there is some controversy going on around this fact, so if you want to you can express your concerns here: #10552 ;)

@paralin
Copy link

paralin commented Aug 11, 2016

How do we do hot module reloading with this requirement?

@iyobo
Copy link

iyobo commented Sep 27, 2016

This is unfortunate. I find Ng2 now becoming more and more complex to build with.
This defeats the whole purpose of DRY. NG2 is supposed to be better than ng1, not worse.

What exactly was wrong with the previous way of doing it?
If this is as a result of lazy loading, then can we have an option to not care about it and simply use the old method?

I can't possibly see my self/team going forward with ng2 if this is the intended direction...
A lot of time has already been wasted dealing with these RC-but-not-really breaking changes, but this one is the straw that breaks the camel's back I'm afraid.

@vicb
Copy link
Contributor

vicb commented Sep 27, 2016

Please add a repro

@slmyers
Copy link

slmyers commented Sep 27, 2016

If it bugs you that much, then you could always declare your routes in your module, so then you don't need 2x import statements.

@IgorMinar IgorMinar added needs reproduction This issue needs a reproduction in order for the team to investigate further and removed needs reproduction This issue needs a reproduction in order for the team to investigate further need: repro labels Oct 4, 2016
@vsavkin
Copy link
Contributor

vsavkin commented Oct 4, 2016

No repro => Closing this issue.

@vsavkin vsavkin closed this as completed Oct 4, 2016
@choeller
Copy link
Contributor Author

choeller commented Oct 9, 2016

@vsavkin @vicb How am I supposed to add a "repro" for a feature that doesn't exist? The problem was described clearly in the first comment of the ticket, so I really don't understand why this got closed without any further statement...

@paralin
Copy link

paralin commented Oct 9, 2016

@choeller it's their flawed way of quickly cleaning up issues so they have less to deal with.

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Oct 9, 2016

It will cause error 'Component declared in more than one module' in paths with children property.
See #10958 . This is the correct solution.

@vicb
Copy link
Contributor

vicb commented Oct 9, 2016

@choeller I was under the impression that you do not need to add your components to the declarations but may be I'm wrong - a repro would help !

@Tragetaschen
Copy link
Contributor

Tragetaschen commented Oct 17, 2016

@vicb

If you don't put the component to the declarations, the error from the AOT compiler is

Error: Cannot determine the module for component MyComponent!

When you remove the DashboardComponent from the AppModule in
https://angular.io/resources/live-examples/toh-5/ts/plnkr.html
the error is

Component DashboardComponent is not part of any NgModule or the module has not been imported into your module [...remaining useless red text cut...]

Given the strange way of organizing code in that plunker, I can see why you would get the impression that declaring the components isn't necessary.

@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
area: router needs reproduction This issue needs a reproduction in order for the team to investigate further
Projects
None yet
Development

No branches or pull requests