Skip to content
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(component-store): Add SelectorObject to select #3629

Merged
merged 9 commits into from
Oct 28, 2022

Conversation

alex-okrushko
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

Adds selectorsObject or an Object that maps properties to selectors (or other Observables) and a new API to the select method.

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #3624

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@netlify
Copy link

netlify bot commented Oct 22, 2022

Deploy Preview for ngrx-io canceled.

Name Link
🔨 Latest commit 8516de7
🔍 Latest deploy log https://app.netlify.com/sites/ngrx-io/deploys/635bda1a42bf37000982e003

@alex-okrushko
Copy link
Member Author

Looks like update of "@nrwl/jest": "14.7.5", #3574 broke our tests.

● Component Store › effect › cancels effect Observable › by unsubscribing with returned Subscription

    TypeError: Cannot read property 'isFake' of undefined

      1436 |
      1437 |     describe('cancels effect Observable', () => {
    > 1438 |       beforeEach(() => jest.useFakeTimers());

@alex-okrushko
Copy link
Member Author

@alex-okrushko
Copy link
Member Author

I'll use legacyFakeTimers for now.

@markostanimirovic
Copy link
Member

@alex-okrushko
Copy link
Member Author

@alex-okrushko

Here is my full suggestion for the select method: https://github.com/markostanimirovic/platform/blob/cs-selectors-object-suggestion/modules/component-store/src/component-store.ts#L257

Let me know what you think :)

I see. Thanks Marko!

I kept your suggestion to keep the operators inlined in the pipe(...), it does have less code, however I kept the original processSelectorArgs helper fn. Let me know if you see any particular advantages in the suggested refactor.

Copy link
Member

@markostanimirovic markostanimirovic left a comment

Choose a reason for hiding this comment

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

🥇

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.

Component-store: Add selector object for selector
4 participants