-
Notifications
You must be signed in to change notification settings - Fork 92
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: support ngOnChanges with correct simpleChange object within rerender #366
Changes from 2 commits
e4acbbd
66ba0c7
4c5a255
d218907
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,14 +123,38 @@ export async function render<SutType, WrapperType = SutType>( | |
|
||
await renderFixture(componentProperties, componentInputs, componentOutputs); | ||
|
||
let renderedPropKeys = Object.keys(componentProperties); | ||
let renderedInputKeys = Object.keys(componentInputs); | ||
let renderedOutputKeys = Object.keys(componentOutputs); | ||
const rerender = async ( | ||
properties?: Pick<RenderTemplateOptions<SutType>, 'componentProperties' | 'componentInputs' | 'componentOutputs'>, | ||
) => { | ||
await renderFixture( | ||
properties?.componentProperties ?? {}, | ||
properties?.componentInputs ?? {}, | ||
properties?.componentOutputs ?? {}, | ||
); | ||
const newComponentInputs = properties?.componentInputs ?? {}; | ||
for (const inputKey of renderedInputKeys) { | ||
if (!Object.prototype.hasOwnProperty.call(newComponentInputs, inputKey)) { | ||
delete (fixture.componentInstance as any)[inputKey]; | ||
} | ||
} | ||
setComponentInputs(fixture, newComponentInputs); | ||
renderedInputKeys = Object.keys(newComponentInputs); | ||
|
||
const newComponentOutputs = properties?.componentOutputs ?? {}; | ||
for (const outputKey of renderedOutputKeys) { | ||
if (!Object.prototype.hasOwnProperty.call(newComponentOutputs, outputKey)) { | ||
delete (fixture.componentInstance as any)[outputKey]; | ||
} | ||
} | ||
setComponentOutputs(fixture, newComponentOutputs); | ||
renderedOutputKeys = Object.keys(newComponentOutputs); | ||
|
||
const newComponentProps = properties?.componentProperties ?? {}; | ||
const changes = updateProps(fixture, renderedPropKeys, newComponentProps); | ||
if (hasOnChangesHook(fixture.componentInstance)) { | ||
fixture.componentInstance.ngOnChanges(changes); | ||
} | ||
renderedPropKeys = Object.keys(newComponentProps); | ||
|
||
fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you also provide a parameter for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have I understood the concept correctly with 4c5a255? I have mixed feelings about it. I want help to provide a nice helpful library. On the other side I am not sure it is a good idea to disable change detection of a component. I just assume the component under test has a design flaw when this is helpful. Or I am just lacking imagination of good use cases.^^ |
||
}; | ||
|
||
const changeInput = (changedInputProperties: Partial<SutType>) => { | ||
|
@@ -360,6 +384,31 @@ function getChangesObj(oldProps: Record<string, any> | null, newProps: Record<st | |
); | ||
} | ||
|
||
function updateProps<SutType>( | ||
fixture: ComponentFixture<SutType>, | ||
prevRenderedPropsKeys: string[], | ||
newProps: Record<string, any>, | ||
) { | ||
const componentInstance = fixture.componentInstance as Record<string, any>; | ||
const simpleChanges: SimpleChanges = {}; | ||
|
||
for (const key of prevRenderedPropsKeys) { | ||
if (!Object.prototype.hasOwnProperty.call(newProps, key)) { | ||
simpleChanges[key] = new SimpleChange(componentInstance[key], undefined, false); | ||
delete componentInstance[key]; | ||
} | ||
} | ||
|
||
for (const [key, value] of Object.entries(newProps)) { | ||
if (value !== componentInstance[key]) { | ||
simpleChanges[key] = new SimpleChange(componentInstance[key], value, false); | ||
} | ||
} | ||
setComponentProperties(fixture, newProps); | ||
|
||
return simpleChanges; | ||
} | ||
|
||
function addAutoDeclarations<SutType>( | ||
sut: Type<SutType> | string, | ||
{ | ||
|
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.
While looking at this PR I think we should also include componentInputs to ngOnChanges.
We don't do this with the current behavior, but I think that that should be added (in a later PR).
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.
Good point. If this PR would be released without it, and I would use componentInputs in rerender, I probably would create a bug for it. If you agree I would already add it in this PR.