8000 feat(): add rxScheduleTask function by Karnaukhov-kh · Pull Request #1633 · rx-angular/rx-angular · GitHub
[go: up one dir, main page]

Skip to content

feat(): add rxScheduleTask function #1633

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Karnaukhov-kh
Copy link
Member

Introduces rxScheduleTask function to schedule tasks with our scheduler.

This is a minimal building block for our concurrent scheduling functionality.

Includes feedback from old PR #1344.

@github-actions github-actions bot added 📚 Docs Web Documentation hosted on github pages 🛂 Test Unit tests, e2e tests, integration tests, test coverage 🛠️ CDK CDK related labels Oct 30, 2023
Karnaukhov-kh and others added 4 commits October 30, 2023 15:06
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
### Default usage

```typescript
import { rxScheduleTask } from '@rx-angular/cdk/render-strategies';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { rxScheduleTask } from '@rx-angular/cdk/render-strategies';
import { rxScheduleTask } from '@rx-angular/cdk/render-strategies';

I don't like it that we export it from render-strategies. Because it doesn't do anything with rendering.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you think about better place? We had this discussion in old PR and decided that this is the best option we currently have. We can say the same about RxStrategyProvider, aren't we? This is a service for scheduling any type of work, but it's still in the render-strategies. Same reasoning goes to rxScheduleTask.

You can make it "related" to rendering by doing rxScheduleTask(() => this.cdRef.detectChanges()) ¯_(ツ)_/¯.

Copy link
Member

Choose a reason for hiding this comment

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

we can introduce a new sub-package for scheduling?

Copy link
Member

Choose a reason for hiding this comment

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

I'd go for it!

Copy link
Member

Choose a reason for hiding this comment

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

hmmm

Copy link
Member

Choose a reason for hiding this comment

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

I would do a rethink before and maybe see what other code we want to move

Copy link
Member Author

Choose a reason for hiding this comment

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

Whatever we decide, please let's not have it frozen for another 1.5 years :D

Copy link
Member

Choose a reason for hiding this comment

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

+1 for scheduling

Karnaukhov-kh and others added 2 commits October 30, 2023 15:12
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Co-authored-by: Enea Jahollari <jahollarienea14@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ CDK CDK related 📚 Docs Web Documentation hosted on github pages 🛂 Test Unit tests, e2e tests, integration tests, test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0