-
-
Notifications
You must be signed in to change notification settings - Fork 176
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(Router): Import RouterTestingModule when stubs are enabled #488
Conversation
@@ -14,15 +14,32 @@ import { RouterLink } from '@angular/router'; | |||
}) | |||
export class RouterLinkDirectiveStub { |
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.
Thanks for the PR. Now that I'm thinking about it, I can't remember why we have a stub here. Can you try to use the original directive, please?
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.
from what I can gather, it looks like I can't just do moduleMetadata.declarations.push(RouterLink);
instead of moduleMetadata.declarations.push(RouterLinkStub);
since angular 9, you need to import the module of the library directly
angular/angular#37047 (comment)
so turns out I need to import RouterTestingModule
along with the other stubs when they are enabled
I was able to make it work but I needed to mock the serializeUrl function in the routerStub, as angular was trying to parse the url using it internally
serializeUrl(): string {
return '/';
},
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'm actually not sure if the routerStub is still needed since I'm now importing RouterTestingModule
, it looks like it should really just stub ActiveRoute
This is added to resolve angular language service errors
It now uses RouterTestingModule in both cases, and adds stubs if set to true
@AnthonyLenglet can you summarize the change, plesae? |
Just updated the initial PR post with a new summary
|
@AnthonyLenglet tests are failing. |
RouterTestingModule is imported, meaning that he is now the one testing this
oh right, I removed those from the jasmine tests, but forgot to remove them from jest, sorry about that ! |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #487
What is the new behavior?
createRoutingFactory
now imports RouterTestingModule when stubs are enabled, then adds the necessary stubs and overrides, thus adding the missing inputs and methods from router related featuresDoes this PR introduce a breaking change?
createRoutingFactory
had an undocumentedmockRouterLink
that has been removed as it is no longer neededOther information