Skip to content

Conversation

@rainerhahnekamp
Copy link
Collaborator

Replaces the computed linkedSignal with the direct resource value, using a Proxy on the get trap to ensure fail-safe access.

- fix withEntityResource
- add additional type tests
- make `undefined value` as the default error handling
@rainerhahnekamp rainerhahnekamp marked this pull request as ready for review January 6, 2026 22:23
Copy link
Collaborator

@michael-small michael-small left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides a call for named withEntityResource tests for error handling, most other comments are nitpicks of jsdocs or pre-existing TODO comments. But all the business logic looks solid.

warningSpy.mockClear();
});

//TODO wait for https://github.com/ngrx/platform/pull/4932 and then add 'value' to the list
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old outstanding TODO

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it belongs in this PR since it would not be possible to backport it to v20. I would recommend doing a separate PR for that. If you want you can do it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No big deal, some other time then

},
);

//TODO wait for https://github.com/ngrx/platform/pull/4932
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old outstanding TODO

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate issue (see above)

});
});

describe('Signature Tests', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs named equivalents right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@rainerhahnekamp rainerhahnekamp added the bug Something isn't working label Jan 8, 2026
Copy link
Collaborator

@michael-small michael-small left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rainerhahnekamp rainerhahnekamp merged commit 369f5a1 into main Jan 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PSA: v21 plan - 20.6.0 works with NgRx v21, Toolkit v21 pending review

4 participants