Skip to content

fix(adapters.nestjs): fix broken reflection when initiate class#80

Merged
omermorad merged 1 commit into
masterfrom
hotfix/nestjs-broken-reflection
Jul 30, 2023
Merged

fix(adapters.nestjs): fix broken reflection when initiate class#80
omermorad merged 1 commit into
masterfrom
hotfix/nestjs-broken-reflection

Conversation

@omermorad

@omermorad omermorad commented Jul 29, 2023

Copy link
Copy Markdown
Collaborator

Description:

In the property injection strategy, in @automock/adapters.nestjs, I was trying to instantiate a class using the new keyword, I encountered an issue where the reflection hooks were attempting to access a property of one of the class dependencies, resulting in a failure. The property did not exist because the class was instantiated with empty parameters.

Example

Here is a code snippet that demonstrates the issue:

Suppose that the decorator @Decorator() tries to use a property from "dep: Dependency"

class Dependency {
  constructor() {}
}

class TargetClass {
  constructor(@Decorator() dep: Dependency) {}
}

// This results in an error
const instance = new TargetClass();

The reflection hooks are attempting to use dep.someProperty, but it's not defined because TargetClass was instantiated without parameters.

Solution

I found that using Object.create instead of the new keyword fixed this issue. Here's an example of how I made the change:

// Create an object with the prototype of TargetClass
const instance = Object.create(TargetClass.prototype);

By using Object.create, the class dependencies were properly initialized, and the reflection hooks were able to access the required properties without errors.

Expected Behavior

The expected behavior is that the class instantiation should handle the dependencies correctly, without resulting in errors when properties are accessed by the reflection hooks.

@codecov-commenter

codecov-commenter commented Jul 29, 2023

Copy link
Copy Markdown

Codecov Report

Merging #80 (a7da233) into master (b7f57a1) will not change coverage.
Report is 6 commits behind head on master.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master      #80   +/-   ##
=======================================
  Coverage   90.84%   90.84%           
=======================================
  Files          11       11           
  Lines         142      142           
  Branches       17       17           
=======================================
  Hits          129      129           
  Misses         13       13           
Flag Coverage Δ
adapters.nestjs 90.84% <100.00%> (ø)
core 90.84% <100.00%> (ø)
jest 90.84% <100.00%> (ø)
sinon 90.84% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...kages/adapters/nestjs/src/class-props-reflector.ts 88.23% <100.00%> (ø)

@omermorad omermorad merged commit 5bb215a into master Jul 30, 2023
@omermorad omermorad deleted the hotfix/nestjs-broken-reflection branch July 30, 2023 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants