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

In app.module place the app.component import last #16598

Open
fireflysemantics opened this issue Jan 8, 2020 · 10 comments
Open

In app.module place the app.component import last #16598

fireflysemantics opened this issue Jan 8, 2020 · 10 comments

Comments

@fireflysemantics
Copy link

@fireflysemantics fireflysemantics commented Jan 8, 2020

In order to make it easier to cut and paste component imports in the router module, Instead of:

import { AppRoutingModule } from './app-routing.module';
import { AppComponent } from './app.component';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { StepperComponent } from './stepper/stepper.component';

Do

import { AppRoutingModule } from './app-routing.module';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { StepperComponent } from './stepper/stepper.component';
import { AppComponent } from './app.component';



@fireflysemantics fireflysemantics changed the title In app.module place the app.component import last in app.module In app.module place the app.component import last Jan 8, 2020
@ngbot ngbot bot modified the milestone: Backlog Jan 9, 2020
@filipesilva
Copy link
Member

@filipesilva filipesilva commented Jan 9, 2020

I think that makes sense. I'd even move the non-local imports all the way to the top.

Would you like to implement this change?

@zadokdaniel
Copy link

@zadokdaniel zadokdaniel commented Jan 15, 2020

hey,
I would like to give it a go.
As a new contributor I would need some guidance if possible :)

I think that makes sense. I'd even move the non-local imports all the way to the top.

Would you like to implement this change?

@filipesilva
Copy link
Member

@filipesilva filipesilva commented Jan 16, 2020

Heya @zadokdaniel, thanks for taking a look!

So this happens on a generator, and generators are relatively easy to test which makes it easier to work on. All generators are in https://github.com/angular/angular-cli/tree/master/packages/schematics, and have unit tests. The unit tests can be ran with yarn test.

The first thing is to figure out which generator does it and reproduce the problem. The issue report didn't actually include that.

@fireflysemantics what was the command you used when you saw this happening? Can you also provide the result of ng version please?

@krishnaanaril
Copy link
Contributor

@krishnaanaril krishnaanaril commented Feb 21, 2020

@filipesilva Hello, What are trying to solve here? In the original issue only app.component is mentioned. Are we trying to move all non-local imports to top?

@filipesilva
Copy link
Member

@filipesilva filipesilva commented Feb 21, 2020

@krishnaanaril yes, that is the idea. The way we add these imports is by reading the source file AST and inserting new imports in the appropriate place. So the algorithm we use to do that would need to handle local/non-local better.

I'm still not sure in what situations this occurs so it's also important to isolate the problematic insertions.

@memark
Copy link

@memark memark commented Mar 21, 2020

Is anyone working on this?

@taylorhutchison
Copy link

@taylorhutchison taylorhutchison commented Mar 25, 2020

I have not encountered this being an issue with other developers. Am I reading this right that this to to make copy/paste easier?

@fireflysemantics
Copy link
Author

@fireflysemantics fireflysemantics commented Mar 27, 2020

Hi - Sorry for the late reply - Just saw the other comments now.

After having done a few more project I've started to always just delete the routing module and place the routes directly into the app module.

This is easier because the app module has to have all the components for declaration anyways, so it's just easier to define the routes there too.

I wonder if creating a separate routing module should be an option on the new command, so that users that want that get that, otherwise the routing will just be generated right into the app.module?

@vinay-9
Copy link

@vinay-9 vinay-9 commented May 7, 2020

--------answer
yes you can do it by using the command as -ng new --routing
->it will create a new project with a separate routing module
I hope that this was your problem, if not then sorry i am just new to github community

@vinay-9
Copy link

@vinay-9 vinay-9 commented May 7, 2020

--------answer
yes you can do it by using the command as -ng new --routing
->it will create a new project with a separate routing module
I hope that this was your problem, if not then sorry i am just new to github community
it is ng new project_name --routing
i dont now how it got removed from my comment

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

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.