fix(@angular-devkit/build-angular): add a base href to karma context #12889
Conversation
This commits adds a base href value in the karma context iframe used to run unit tests. It solves a very old issue in Angular angular/angular#12295 where a unit test throws: No base href set. Please provide a value for the APP_BASE_HREF token or add a base element to the document. even if the application is fine. This is because the `index.html` from Angular CLI contains a base href value, but not the Karma context iframe. So when adding a unit test with a testing module that imports a NgModule, for example `AppModule`, which itself imports `RouterModule`, the unit test used to throw an error (regression appeared in router 3.1). That could be solved by either adding `RouterTestingModule` to the testing module, or by adding a provider `{ provide: APP_BASE_HREF, useValue: '/' }`, but required to understand the issue (see how many thumbs up there are on the original issue). This solves the issue in a transparent way: developers won't even encounter the problem anymore.
There was a problem hiding this comment.
I agree that this is a confusion situation, but am not 100% sure if this is the right approach...
We do add <base href="/"> to the default index so that part should be a default in the karma context too, I agree.
But that baseHref is configurable on builds, while it isn't on test. So there are side effects that could conceivably come from this change.
I can't think of any concrete ones right now so I think it's better if we add it in for consistency with the default index.html.
alan-agius4
added
the
target: patch
…12889) This commits adds a base href value in the karma context iframe used to run unit tests. It solves a very old issue in Angular angular/angular#12295 where a unit test throws: No base href set. Please provide a value for the APP_BASE_HREF token or add a base element to the document. even if the application is fine. This is because the `index.html` from Angular CLI contains a base href value, but not the Karma context iframe. So when adding a unit test with a testing module that imports a NgModule, for example `AppModule`, which itself imports `RouterModule`, the unit test used to throw an error (regression appeared in router 3.1). That could be solved by either adding `RouterTestingModule` to the testing module, or by adding a provider `{ provide: APP_BASE_HREF, useValue: '/' }`, but required to understand the issue (see how many thumbs up there are on the original issue). This solves the issue in a transparent way: developers won't even encounter the problem anymore.
|
@cexbrayat @vikerman |
|
@UliOnline would you be interested in submitting a similar PR? |
same fix as in angular#12889 , but for the karma debug context. Issue described in the above PR happens again, if you switch to the karma debug runner. Karma will now use the karma-debug.html instead of the karma-context.html, rendering the initial fix useless.
|
@filipesilva i tried so, but realized the following: In case someone else is having the same issue, i just decided to provide a base ref in the corresponding component test directly so the tests work in any case. |
|
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. |
This commits adds a base href value in the karma context iframe used to run unit tests.
It solves a very old issue in Angular angular/angular#12295 where a unit test throws:
even if the application is fine. This is because the
index.htmlfrom Angular CLI contains a base href value, but not the Karma context iframe. So when adding a unit test with a testing module that imports a NgModule, for exampleAppModule, which itself importsRouterModule, the unit test used to throw an error (regression appeared in router 3.1).That could be solved by either adding
RouterTestingModuleto the testing module, or by adding a provider{ provide: APP_BASE_HREF, useValue: '/' }, but required to understand the issue (see how many thumbs up there are on the original issue).This solves the issue in a transparent way: developers won't even encounter the problem anymore.