-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
bugfix(schematics): fix for angular 12 and align schematics #437
bugfix(schematics): fix for angular 12 and align schematics #437
Conversation
* @param raw initial object | ||
* @param keys list of keys to remove from object | ||
*/ | ||
function _omit<T extends Record<string, any>>(raw: T, keys: (keyof T)[]): any { |
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. I think this function is doing the opposite of what you want it to!
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.
@benelliott is right. Check your filter logic.
@@ -23,6 +23,7 @@ | |||
"angular testing dumb components" | |||
], | |||
"dependencies": { | |||
"@angular-devkit/core": ">= 10.1.0", |
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.
Why do we need it?
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.
Wenn I tried to run this as a linked package it complained that @angular-devkit/core was not found although it was installed in the project.
@@ -21,7 +21,7 @@ import { ComponentOptions, DirectiveOptions, ServiceOptions } from './schema'; | |||
export function spectatorComponentSchematic(options: ComponentOptions): Rule { | |||
return chain([ | |||
externalSchematic('@schematics/angular', 'component', { | |||
...options, | |||
..._omit(options, ['jest', 'withHost', 'withCustomHost']), |
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.
Can you explain, 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.
Angular 12 complains about options that are passed but not known
* @param raw initial object | ||
* @param keys list of keys to remove from object | ||
*/ | ||
function _omit<T extends Record<string, any>>(raw: T, keys: (keyof T)[]): any { |
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.
@benelliott is right. Check your filter logic.
Thanks for the PR. See my comments, and please check that it works. |
cf640dd
to
584bdfd
Compare
I fixed the filter logic, but I'm not able to test because build is broken.
|
584bdfd
to
3d1b088
Compare
@DaSchTour Can you clarify about the build being broken? Are you waiting on another issue to be resolved or just stuck on this fix because your changes broke the build and you don't know how to fix it? |
@cmplank I actually have now idea what this build issue is about. For me it looks completely unrelated. But I have no idea how to fix it and why it breaks. I just checked, the same issue is there on master. So it's not related to my changes. |
There is a PR |
Yeah, I see. Thanks. |
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: #434
What is the new behavior?
Added the type property and also improved the behavior for angular 12 which validates the input and breaks on unkown options
Does this PR introduce a breaking change?
Other information
Sadly I couldn't test it locally because the build broke because of some rxjs issues in ActivatedRouteStub