-
Notifications
You must be signed in to change notification settings - Fork 12k
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(@schematics/angular): remove useless import for Ivy #11874
Conversation
@@ -1,4 +1,4 @@ | |||
import { BrowserModule } from '@angular/platform-browser'; | |||
<% if (!experimentalIvy) { %>import { BrowserModule } from '@angular/platform-browser';<% } %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is the same problem for AppRoutingModule
, as at the moment it looks like this is only being used if it's not experimentalIvy
angular-cli/packages/schematics/angular/application/other-files/app.module.ts
Lines 11 to 13 in 151a0b2
imports: [<% if (!experimentalIvy) { %> | |
BrowserModule<% if (routing) { %>, | |
AppRoutingModule<% } %> |
I think that AppRoutingModule
shouldn't be effect by the fact if experimentalIvy
is enabled or disabled.
That said, maybe @hansl can confirm about this, as he initial working on implementing the experimentalIvy
support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I generated it without the routing
flag so I didn't see it, but I can add it if @hansl confirms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I don't even know if we support router in Ivy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I amended the commit to remove the AppRoutingModule
if the experimentalIvy
flag is used.
Using cli `6.2.0-beta.2` with the new `experimentalIvy` flag leads to: ERROR in src/app/app.module.ts(1,1): error TS6133: 'BrowserModule' is declared but its value is never read.
151a0b2
to
aae4e75
Compare
We should not promote unused variable to a compiler error. Do we have |
@alexeagle no worries, it's not in the default tsconfig. Sorry if my description was misleading: I just wanted to point out that the useless imports should be removed. |
Rerunning failed tests... |
Please assign appropriate merge target |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Using cli
6.2.0-beta.2
with the newexperimentalIvy
flag leads to: