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(template): introduce rxChunk #1380

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

Conversation

hoebbelsB
Copy link
Member

ChunkDirective

This PR introduces the *rxChunk structural directive.

the *rxChunk directive serves as a convenient way for dividing template work into chunks.
Applied to an element, it will schedule a task with the given RxRenderStrategy in order to postpone the template creation of this element.

rx-chunk

PR Checklist

  • implementation
  • demo
  • unit tests
  • docs (jsdocs)

Features of *rxChunk

  • lightweight alternative to *rxLet="[]"
  • renderCallback
  • no value binding
  • no context variables
  • zone agnostic
  • suspense template

long task

distribute work

@nx-cloud
Copy link

nx-cloud bot commented Jul 25, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b3c6d22. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@github-actions github-actions bot added the </> Template @rx-angular/template related label Jul 25, 2022
Copy link
Member

@Karnaukhov-kh Karnaukhov-kh left a comment

Choose a reason for hiding this comment

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

Hi @hoebbelsB really like the idea. Looks super interesting!

* @param {NextObserver<void>} renderCallback
*/
@Input('rxChunkRenderCallback')
set renderCallback(renderCallback: NextObserver<void>) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use not only subjects but also just a function? I know it's more like a general question for render callbacks but would it worth the try?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea, I would even more like to just have an @Output. But it looks like it's not supported, at least for the * syntax. angular/angular#12121

*
* @param { RxStrategyNames<string> } strategy
*/
@Input('rxChunk') strategy = this.strategyProvider.primaryStrategy;
Copy link
Member

Choose a reason for hiding this comment

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

Is it planned to work with not concurrent strategies?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, but why would u? it basically makes the rxChunk do nothing useful anymore :D

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's the thing. Can we type this input to RxConcurrentStrategyNames?

Copy link
Member

Choose a reason for hiding this comment

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

We have to support all Strategien. Other strategies then concurrent will have an effect definitely. U could e.g. use 'noop' strategy when out of viewport

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a need to support all strategies. If you want to change the rendering strategy conditionally, then this is necessary.

And it makes sense in many cases to switch the strategies based on conditions like first render and other things which may affect CLS

libs/template/chunk/src/lib/tests/chunk.directive.spec.ts Outdated Show resolved Hide resolved
@edbzn
Copy link
Member

edbzn commented Aug 5, 2022

@hoebbelsB shouldn't we wait for the first stable release before introducing new elements?

@BioPhoton
Copy link
Member

As not all strategies chunk let's call it rxTemplate?

@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #1380 (dc48688) into main (ec364d8) will increase coverage by 10.19%.
The diff coverage is 88.46%.

❗ Current head dc48688 differs from pull request most recent head b3c6d22. Consider uploading reports for the commit b3c6d22 to get more accurate results

@@             Coverage Diff             @@
##             main    #1380       +/-   ##
===========================================
+ Coverage   74.95%   85.14%   +10.19%     
===========================================
  Files         151       20      -131     
  Lines        2951      404     -2547     
  Branches      531       56      -475     
===========================================
- Hits         2212      344     -1868     
+ Misses        617       34      -583     
+ Partials      122       26       -96     
Flag Coverage Δ
cdk ?
state ?
template ?

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

Impacted Files Coverage Δ
libs/template/chunk/src/lib/chunk.directive.ts 86.95% <86.95%> (ø)
libs/template/chunk/src/lib/chunk.module.ts 100.00% <100.00%> (ø)
libs/template/let/src/lib/let.directive.ts 81.25% <0.00%> (-17.12%) ⬇️
libs/template/for/src/lib/for.directive.ts 94.91% <0.00%> (-1.64%) ⬇️
...render-strategies/src/lib/concurrent-strategies.ts
...nfigurations/src/lib/model/configurations.types.ts
...ibs/cdk/zone-less/rxjs/src/observable/fromEvent.ts
libs/cdk/transformations/src/lib/object/toggle.ts
libs/cdk/template/src/lib/render-error.ts
libs/cdk/zone-less/rxjs/src/scheduler/scheduler.ts
... and 127 more

@ChristopherPHolder
Copy link
Contributor

Hi, the current workaround for this is to do *rxLet="[]"

Which from anyone who is not aware of the pattern makes 0 sense.

Is there anything blocking the issue?

Is it only the name that's blocking?

*rxChunk - *rxTemplate - *rxRender

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog 🧊 </> Template @rx-angular/template related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants